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

Fix #408: Response.age is semantically a timedelta, not a datetime #414

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@cvrebert
Copy link
Contributor

commented Jul 20, 2013

First time I've tried to contribute to Werkzeug; criticism is welcome.
Fixes #408.

# but disregarding fractional seconds
age = age.seconds + (age.days * 24 * 3600)

age = int(age)

This comment has been minimized.

Copy link
@cvrebert

cvrebert Jul 20, 2013

Author Contributor

I'm uncertain what level of duck-typing vs. strong-typing Werkzeug observes. This arguably should be:

if not isinstance(age, integer_types): raise TypeError(...)

which would catch floats, Decimals, etc., but would also prohibit custom int-like/timedelta-like types.
Feedback on this point would be much appreciated.

@cvrebert

This comment has been minimized.

Copy link
Contributor Author

commented Jan 24, 2014

Ping. Is there anything besides potentially the naming that's holding this back?

@k-nut

This comment has been minimized.

Copy link

commented Jan 25, 2014

Have a look at issue #480. @mitsuhiko seems to be quite busy so there is not much merging going on right now. I find it sad though since this prevents good features and even relatively important bug fixes to find their way into the project...

@untitaker untitaker force-pushed the pallets:master branch 2 times, most recently from 8377106 to 1842d71 Aug 25, 2014

@untitaker

This comment has been minimized.

Copy link
Member

commented Aug 31, 2014

Not sure of the implications of this. I think it'd be enormouse API breakage to just include this into 0.10.

@cvrebert

This comment has been minimized.

Copy link
Contributor Author

commented Aug 31, 2014

The current behavior is already very broken; the Age header in the HTTP response has a completely invalid value!

@untitaker untitaker force-pushed the pallets:master branch 2 times, most recently from 16edc59 to afc2c67 Sep 4, 2014

@untitaker untitaker force-pushed the pallets:master branch from 673b84b to cbd049d Sep 29, 2014

@untitaker untitaker force-pushed the pallets:master branch from 29bfca9 to 57411e3 Oct 11, 2014

@untitaker untitaker force-pushed the pallets:master branch 2 times, most recently from f9d0f4e to cbec4d1 Oct 25, 2014

@untitaker untitaker force-pushed the pallets:master branch from bc0e006 to 96e70d0 Jan 25, 2015

@untitaker

This comment has been minimized.

Copy link
Member

commented Jan 28, 2015

FWIW, I've decided to delay this to 1.0, because we can cleanly break the API in that release. The patch itself seems to be fine and can be applied as-is.

@untitaker untitaker added this to the 1.0 milestone Jan 28, 2015

@untitaker untitaker added the bug label Jan 28, 2015

@davidism

This comment has been minimized.

Copy link
Member

commented Apr 12, 2017

@untitaker can we re-schedule this to 0.13 since it's been 2 years since 1.0 was added? I think this is ok for a feature release.

@untitaker

This comment has been minimized.

Copy link
Member

commented Apr 12, 2017

@davidism

This comment has been minimized.

Copy link
Member

commented Apr 12, 2017

Ah, I didn't know we were following semver, I just figured we were ok making bigger changes in second-number releases.

@davidism davidism modified the milestones: 0.13, 1.0 Apr 12, 2017

@untitaker

This comment has been minimized.

Copy link
Member

commented Apr 12, 2017

@davidism

This comment has been minimized.

Copy link
Member

commented Apr 12, 2017

#1104 rebases this against master so that it can be cleanly merged.

@davidism davidism closed this Apr 12, 2017

davidism added a commit that referenced this pull request Apr 13, 2017

Merge pull request #1104 from davidism/age-fix
Continue #414: Fix #408: Response.age is semantically a timedelta, not a datetime
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.