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

backport config.add_cache_buster #2186

Merged
merged 2 commits into from
Dec 18, 2015
Merged

Conversation

mmerickel
Copy link
Member

Backporting changes from #2171 into 1.6.

mmerickel added a commit to mmerickel/pyramid that referenced this pull request Dec 18, 2015
mmerickel added a commit that referenced this pull request Dec 18, 2015
backport config.add_cache_buster
@mmerickel mmerickel merged commit 7c7ab53 into Pylons:1.6-branch Dec 18, 2015
@sbrunner
Copy link

I try to upgrade pyramid without any success...
I have this kind of code

version = uuid.uuid4().hex
def version_cache_buster(request, subpath, kw):
    return (version + "/" + subpath), kw

config.add_static_view(name='static', path='mypackage:static')
config.add_cache_buster('mypackage:static', version_cache_buster)
config.override_asset('mypackage:static/[a-f0-9]+/', 'mypackage:static/')

But the [a-f0-9]+ in the asset override don't work, what should I put here?

@mmerickel
Copy link
Member Author

@sbrunner override_asset does not support regexes. What exactly are you attempting to accomplish here? If the files are already in the folder then they will be served normally by the add_static_view attached to mypackage:static.

@sbrunner
Copy link

Then this mean that with pull request it's not anymore possible to have a cache buster based on a folder, the only one who relay fix the cache issue...

@mmerickel
Copy link
Member Author

Sorry I honestly have no idea what you're saying. Cache busters are tied to folders. Your original code with the override_asset makes no sense. However this code without the override_asset makes a lot of sense. When you use this code what is the problem you are seeing?

version = uuid.uuid4().hex
def version_cache_buster(request, subpath, kw):
    return (version + "/" + subpath), kw

config.add_static_view(name='static', path='mypackage:static')
config.add_cache_buster('mypackage:static', version_cache_buster)

If the files are not actually named <version>/<file> on disk, then you will still get a 404 because cache busting does not solve issues with serving the assets... only generating the urls. Perhaps this is not as clear as it could be in the docs.

@sbrunner
Copy link

Ok, I understand and effectively with the asset override I try to fix the 404 error...

We solved our problem precariously with the following simple cache buster:

class VersionCacheBuster(PathSegmentMd5CacheBuster):
    def tokenize(self, pathspec):
        return get_cache_version()

Where the get_cache_version() returns a deferent string on each application start.

Then when we use the get_static_url we get something like .../<cache_version>/<desired_path>.
And when we call on .../<whatever>/<desired_path> we get the desired_path.

Wow to do it with the new version?

If it's not possible for us it's a big regression (we use the alpha release just for the path segment based cache buster...).
We use it because the cache buster based on a query string is not a good solution for the css files. They will be static files and they referrer some image without simple possibility add the query string. Then PathSegment*CacheBuster will be a grate solution.

@chrisrossi you made the original version of the cache control, what do you think about it?

It it's not anymore possible we should find an other solution outside pyramid...

@stevepiercy
Copy link
Member

@sbrunner did you read the new section in the docs on Cache Busting? (Note the version number in the URL.) There are examples for both query string and path segment cache busting, as well as a discussion of various strategies.

@digitalresistor
Copy link
Member

@sbrunner The match() concept that turned cachebusted URL's back into non-cachebusted paths was extremely fragile and unless used in a very specific manner it would break for all but the simplest use cases, and did not work correctly with asset overrides.

To solve your issue, you could write a tween that turned the cache busted URL back into the non-cache busted URL before it entered the routing machinery. This will depend highly on your cache buster and what it does with the path.

At the moment we consider the static view turning the cache busted URL's back into non-cache busted URL's outside of the scope of Pyramid due to the complexities surrounding it.

@digitalresistor
Copy link
Member

@stevepiercy At the moment we don't document how to go from cache busted back to non-cache busted so that the static view can do it's thing. It's an assumption that the path returned from the cachebuster will match a physical file on disk.

Thing is, there is no good way to do the reverse without dictating very specific ways in which a cachebuster could do it's work, something we avoided doing. Many people use asset pipelines that will modify the file names on disk to match the cachebusted name, and they would have no such problems.

@sbrunner
Copy link

Thanks @bertjwregeer I just make something working :-)

@stevepiercy
Copy link
Member

@bertjwregeer I had asset pipelines in mind, as discussed on the 1.6-branch cachebusting docs. But, yeah, probably not what @sbrunner wanted.

@sbrunner would you be willing to write up a reproducible example of what you did to make it work for you? It might be a good Pyramid Cookbook recipe. I help out with writing docs.

@sbrunner
Copy link

@stevepiercy

What I finally do is something like

version = uuid.uuid4().hex
def version_cache_buster(request, subpath, kw):
    return (version + "/" + subpath), kw

class CachebusterTween:
    """ Get back the cachebuster URL. """
    def __init__(self, handler, registry):
        self.handler = handler

    def __call__(self, request):
        path = request.path_info.split("/")
        if path[1] in ["static"]:
            # remove the cache buster
            path.pop(2)
            request.path_info = "/" .join(path)
        return self.handler(request)

def includeme(config):
    ...
    config.add_static_view(name='static', path='mypackage:static')
    config.add_cache_buster('mypackage:static', version_cache_buster)
    config.add_tween("myapp.CachebusterTween")

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

Successfully merging this pull request may close these issues.

4 participants