Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cachebust_match argument not documented; not allowed by add_static_view #2675

Closed
ztane opened this issue Jul 4, 2016 · 8 comments
Closed
Milestone

Comments

@ztane
Copy link
Contributor

ztane commented Jul 4, 2016

We are generating one path segment cachebreaker for all of our static files (our static files do not really change that often, only in release cycles, thus it is OK to reload all of them whenever a new release occurs); this allows us to use relative urls without manifests and allows things like require.js work painlessly as well; but for this we utilized a modified PathSegmentCacheBuster.

Now that the new cachebusting scheme has come, it is not possible to do this any longer, since add_static_view doesn't like seeing cache tokens any more.

static_view seems to support cachebust_match which could be used to remove the busting token from the subpath, but add_static_view thinks an argument by that name is a custom predicate instead.

@ztane
Copy link
Contributor Author

ztane commented Jul 5, 2016

And if it wasn't clear, what we've been doing is serving all the static files at

 /static/<application-git-revision-id>/

so that the relative urls in CSS and more importantly JavaScript as we're using on-demand loading for various frameworks, including loading locales for libraries and such, can continue to work with relative URLs, and we have to rewrite absolutely nothing to get cache busting work correctly everywhere, and every file, including that 16x16 pixel logo added that included css will also be cachebusted, ensuring that nothing needs to be flushed in CDN.

@digitalresistor
Copy link
Member

Unfortunately we had to remove the ability for the static views to remove cache busting tokens because it is impossible to do so sanely.

See my comment here: #2186 (comment)

Also see this comment: #2171 (comment)

I am not sure why cachebust_match still exists. Maybe @mmerickel can shed some light on that, but at this point in time there is no support for going from a cache buster URL to a non-cache busted asset spec.

Your best bet is to write a tween that re-writes the URL on the way in.

@digitalresistor
Copy link
Member

Confirmed with @mmerickel on IRC that cachebust_match was left-over from when that feature was ripped out, and needs to be excised from the codebase.

@mmerickel
Copy link
Member

That feature was intended to be removed, however it looks like the static_view support slipped through the cracks. That is not to say that your use-case is invalid, or shouldn't be discussed.

The main reason this is not solved generally in Pyramid (right now) is that we decided not to make any assumptions about the URL that would be generated by your cachebuster. This means it may not be mounted in /static (the url that is being matched by add_static_view). The cache buster handles ONLY generating busted urls and we left the matching up to the user as we have no idea how the urls will be generated. The way to solve this (right now) is to add a tween which can match busted urls and rewrite them back to their original version. This is easy to do for a case such as yours. Here is an example that I have used in a real app to match urls of the format /static/<asset>-<token>.<ext>. https://gist.github.com/mmerickel/61023515229b6645e80d

I hope this will a) help you solve your problem and b) give you some fodder for discussing this further if we are to solve it in Pyramid. The comments linked above by @bertjwregeer are still relevant.

@digitalresistor
Copy link
Member

I've created a PR that removes cachebust_match: #2681

@mmerickel
Copy link
Member

@ztane Your issue is easily solved by simply not using the cache busting framework. A global cache busting token is quite trivial:

config.add_static_view('static/' + token, 'myapp:static', http_cache=10 * 365 * 24 * 3600)

# let's redirect other requests to tokenized versions
config.add_route('redirect-static', '/static/*subpath')
def static_redirect_view(request):
    next_url = request.static_url('myapp:static/' + '/'.join(request.subpath), _query=request.GET)
    return HTTPFound(next_url)
config.add_view(static_redirect_view, route_name='redirect-static')

@ztane
Copy link
Contributor Author

ztane commented Jul 5, 2016

Yeah that was what we were using in pyramid 1.5.

@mmerickel mmerickel added this to the 1.8 milestone Jul 17, 2016
@mmerickel
Copy link
Member

We merged #2681 which removes this undocumented argument. You'll need to come up with another way to serve the cache busted urls. Right now pyramid only helps generate them. Several possible solutions were mentioned already in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants