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

Various TTL fixes #6

Merged
merged 5 commits into from Nov 1, 2013
Merged

Various TTL fixes #6

merged 5 commits into from Nov 1, 2013

Conversation

DavidBarts
Copy link
Contributor

  1. Utility.parse_time was ignoring time zones. This was causing robots.txt to sometimes get fetched excessively.
  2. The no-cache Cache-Control: directive should only be honored if it appears "bare", not in no-cache=blah form (in which case it has another meaning).
  3. If the expires header cannot be parsed, use the default TTL instead of a TTL of 0.

@ghost ghost assigned dlecocq Oct 1, 2013
@dlecocq
Copy link
Contributor

dlecocq commented Oct 15, 2013

Seems reasonable to me, but looks like there are a couple tests that need fixing first.

'''Parse a human-readable time into a timestamp'''
return time.mktime(dateutil.parser.parse(strng).timetuple())
'''Parse an HTTP-style (i.e. email-style) time into a timestamp'''
v = email.utils.parsedate_tz(strng)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the reason for this switch a matter of strictness? Or to remove the dateutil dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dateutil was ignoring time zones and parsing everything as GMT. This was causing robots.txt data to expire too soon or too late, depending on the time zone.

@DavidBarts
Copy link
Contributor Author

OK, the tests pass now. Fixed them to accommodate the changes I made. Coverage is not 100% but it's the same as before I made my edits.

dlecocq pushed a commit that referenced this pull request Nov 1, 2013
@dlecocq dlecocq merged commit 84ec77f into master Nov 1, 2013
@dlecocq dlecocq deleted the ttl-fix branch November 1, 2013 18:28
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