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

Determanistic url params #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

matt-oakes
Copy link

The URL parameters should be deterministic as the full URL is used as the key for caching. If the non-ordered dictionary is used the caching will miss even when a value has been stored.

I've also converted the max age from what I assume is milliseconds into seconds, which Python requires.

The Prismic API is sending very high expiry times (10 years) for articles, which makes it seem like it's using milliseconds rather than seconds. Some caching libraries (including AppEngine Memcache) sees this 10 year value as an error and refuses to store or retrieve the cached value.

@matt-oakes
Copy link
Author

Is there any update on this? Currently we're using a forked version of this library to work around this issue.

@erwan
Copy link
Contributor

erwan commented Feb 5, 2016

Hi,

I'm fine for the deterministic order, but I don't want to change milliseconds to seconds because it would break existing things and be inconsistent with API in other languages.

Could you separate them?

The URL parameters should be deterministic as the full URL is used as the key for caching. If the non-ordered dictionary is used the caching will miss even when a value has been stored.
@matt-oakes
Copy link
Author

That should be just the URL change now. Once this is merged in, can you push a release please.

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.

None yet

2 participants