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

[5.2] Fix FileStore expiration #13708

Merged
merged 1 commit into from
May 27, 2016
Merged

[5.2] Fix FileStore expiration #13708

merged 1 commit into from
May 27, 2016

Conversation

bairdj
Copy link
Contributor

@bairdj bairdj commented May 26, 2016

Specifying a float cache expiration breaks the FileStore cache, as the unserialize call in the getPayload method fails if the timestamp is greater than 10 characters. This PR prevents this by casting the expiration to int to ensure a 10 character timestamp

@vlakoff
Copy link
Contributor

vlakoff commented May 26, 2016

File cache only expects full minutes (probably seconds would has been a better choice, but that's another story).

Your PR just rounds down the number of minutes: it doesn't add granularity, so it doesn't really improve the situation. This might also bring bugs elsewhere.

@GrahamCampbell
Copy link
Member

Perhaps better to throw an exception if we overflowed.

@vlakoff
Copy link
Contributor

vlakoff commented May 26, 2016

I'm not sure such an argument check is worth it. A number of minutes is expected, this is discussable, but for now deal with it.

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