Skip to content

Conversation

abathur
Copy link
Contributor

@abathur abathur commented Mar 1, 2019

Instead of requiring JWT_ACCESS_TOKEN_EXPIRES and JWT_REFRESH_TOKEN_EXPIRES to
be of type datetime.timedelta or False, checks in config.py support any
value that can be successfully added to a datetime.datetime object.

Closes #214.

(I'm not well-configured to test this atm, so I'm relying on CI to slap me down if there's a problem here.)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling ef2ecbd on abathur:ducktype_expires into 3f37e1e on vimalloc:master.

@coveralls
Copy link

coveralls commented Mar 1, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling d632ad3 on abathur:ducktype_expires into 63075d3 on vimalloc:master.

@abathur abathur force-pushed the ducktype_expires branch from 65c810a to a5a425f Compare March 2, 2019 02:29
@vimalloc
Copy link
Owner

vimalloc commented Mar 2, 2019

Nice 👍 Could you add a unit test for the dateutil.relativedelta? You can include that package in the requirements.txt.

@abathur
Copy link
Contributor Author

abathur commented Mar 2, 2019

I was just about to ask; I wasn't sure if you'd want the extra dependency or not. :)

@vimalloc
Copy link
Owner

vimalloc commented Mar 2, 2019

For sure, thanks for checking 👍 Having the extra development dependency doesn't bother me at all.

Thanks for contributing!

@vimalloc
Copy link
Owner

vimalloc commented Mar 2, 2019

Oh, that reminds me. With the added requirement of the six package, could you also update the install requirements here (https://github.com/vimalloc/flask-jwt-extended/blob/master/setup.py#L31) to list six as well?

Cheers.

@vimalloc
Copy link
Owner

vimalloc commented Mar 2, 2019

Whoops, I totally forgot that tox used it's own dependencies as well. Gotta love dependency management right?

If you add six and dateutils to the tox.ini I expect these tests should pass 👍

@vimalloc
Copy link
Owner

vimalloc commented Mar 2, 2019

Actually, only dateutil. Six will be installed because it's part of the setup.py now 👍

@abathur
Copy link
Contributor Author

abathur commented Mar 2, 2019

I think I'm missing something about your test setup; dateutil imports were failing with dateutil in requirements.txt, but started working when I added it to deps in tox.ini.

edit: look what i get for posting without scrolling up :)

Instead of requiring JWT_ACCESS_TOKEN_EXPIRES and JWT_REFRESH_TOKEN_EXPIRES to
be of type `datetime.timedelta` or `False`, checks in config.py support any
value that can be successfully added to a `datetime.datetime` object.

Closes vimalloc#214.
@abathur abathur force-pushed the ducktype_expires branch from afc23c6 to 076789a Compare March 2, 2019 04:42
@vimalloc
Copy link
Owner

vimalloc commented Mar 2, 2019

Yeah, the requirements.txt contains a bunch of additional stuff that isn't needed for unit tests (such as the code to generate documentation or push up new releases to PyPI). For the sake of speed, I have tox setup to only install the necessary requirements for running the unit tests instead of all the code in the requirements.txt.

Thanks for spending the time to put this together, it looks good 👍

@vimalloc vimalloc merged commit 057904c into vimalloc:master Mar 2, 2019
@vimalloc
Copy link
Owner

vimalloc commented Mar 2, 2019

This has been released in 3.18.0. Cheers.

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.

3 participants