Respond directly with badges rather than redirecting #4559
Conversation
|
Hmmm tests are failing if you don't run |
| else: | ||
| url = badge_path % 'failing' | ||
|
|
||
| with staticfiles_storage.open(url) as fd: |
There was a problem hiding this comment.
Is there a way to put a timeout on this call? We used to do this exact same thing with shields.io, but whenever they had downtime, it took down our entire web stack because we'd end up just spinning our web processes waiting for badges.
There was a problem hiding this comment.
We weren't doing 302s to shields.io? I knew we used to use shields.io but I didn't look into precisely how it was implemented. Doing redirects seems like it wouldn't rely on them being up. Badges would be down when shields.io is down but it wouldn't affect our servers at all.
There was a problem hiding this comment.
It looks like #711 introduced shields.io. It introduced it by streaming the HTTP response from shields which is definitely brittle as you describe. When they're down, it would cause problems for us unless we specify timeouts correctly.
However, it looks like things switched to redirecting to shields.io (in this commit 47757de which doesn't really have a PR). That seems like a step in the right direction for sure. #3057 swapped shields.io for self-hosted SVGs but there's no information in the PR as to why that was desirable. To me it seems like a step back.
There was a problem hiding this comment.
Is there a way to put a timeout on this call?
To answer this question directly, I don't see a way to add a specific timeout to this call using the version of the Azure libraries we are using. The default timeout is long too (65 seconds).
There was a problem hiding this comment.
I believe shields had a decent amount of downtime.
There was a problem hiding this comment.
Looks like this is the issue ticket about it: #2065
| with staticfiles_storage.open(url) as fd: | ||
| return HttpResponse( | ||
| fd.read(), | ||
| content_type='image/svg+xml', |
There was a problem hiding this comment.
Isn't here the place where we want to add some specific cache headers for lowering down the cache time?
There was a problem hiding this comment.
The method itself has the @never_cache decorator which handles that.
There was a problem hiding this comment.
Perfect! I'm sorry, I didn't see it.
|
I'm going to close this PR in favor of #4558. After discussion, I think that PR coupled with getting the cache-control headers right on the CDN is the best approach. |
This changes the docs badges to be served directly from Django. This is less efficient but gives us precise caching control and centralizes all the cache control in one place.
One possible optimization is to cache the part where we read from storage. The "passing" graphic doesn't change frequently. However, it is possible that reading from cache isn't really faster than reading from storage.
If merged, #4558 can be closed.
Fixes #4557.