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

Add support for PyJwt_2_0_0 #206

Merged
merged 13 commits into from Jan 5, 2021
Merged

Add support for PyJwt_2_0_0 #206

merged 13 commits into from Jan 5, 2021

Conversation

rahulraina7
Copy link
Contributor

Goal

PyJWT just released v2.0.0 which breaks sanic-jwt due to jpadilla/pyjwt#529

Approach

Type checking before returning access token.

@ahopkins
Copy link
Owner

Looks like I need to fix the Travis builds. Will look into that next week and get this merged. Thanks for the PR

Copy link
Owner

@ahopkins ahopkins left a comment

Choose a reason for hiding this comment

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

Can you also update the requirements?

@rahulraina7
Copy link
Contributor Author

Can you also update the requirements?

Updated setup.py

ahopkins
ahopkins previously approved these changes Dec 28, 2020
@ahopkins
Copy link
Owner

Can you take a look at the failing tests?

@codecov
Copy link

codecov bot commented Dec 29, 2020

Codecov Report

Merging #206 (eb31e97) into master (db4eeb4) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #206   +/-   ##
=======================================
  Coverage   97.71%   97.71%           
=======================================
  Files          12       12           
  Lines        1049     1050    +1     
  Branches      174      174           
=======================================
+ Hits         1025     1026    +1     
  Misses         15       15           
  Partials        9        9           
Impacted Files Coverage Δ
sanic_jwt/authentication.py 93.86% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86ad444...eb31e97. Read the comment docs.

@rahulraina7
Copy link
Contributor Author

Hi @ahopkins: I have fixed the tests. The suite is failing due to some issues with coverage, probably some configuration issues. I will try to have a look but if you know what this is about then that's better

@ahopkins
Copy link
Owner

ahopkins commented Dec 29, 2020

Awesome. I am not too worried about the codecov. Thanks for this I will get the checks sorted out and merge.

image

@rahulraina7
Copy link
Contributor Author

I have commented out the coverage combine command for now, since there aren't multiple coverage files to combine.

@stikks
Copy link

stikks commented Jan 5, 2021

Happy new year guys. Any idea when this PR might get merged into master and released.

@rahulraina7
Copy link
Contributor Author

@ahopkins : I am freezing this project dependency on sanic - 20.9.1. Due to sanic-org/sanic#1989 for now. Either I can make those changes you suggested there or wait for sanic for the patch.

@ahopkins
Copy link
Owner

ahopkins commented Jan 5, 2021

I am pushing out a patch for that shortly.

@ahopkins
Copy link
Owner

ahopkins commented Jan 5, 2021

See #1993. Will also merge to release as 20.12.1

Copy link
Owner

@ahopkins ahopkins left a comment

Choose a reason for hiding this comment

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

Small change to overcome the name registry and then ready to merge.

tox.ini Outdated
@@ -24,7 +24,7 @@ deps =
pytest-flakes
pytest-pep8
pytest-travis-fold
sanic
sanic==20.9.1
Copy link
Owner

Choose a reason for hiding this comment

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

I am not going to merge this with a 20.9 dependency.

Just add this line to the top of conftest.py after the imports.

Sanic.test_mode = True

Copy link
Owner

Choose a reason for hiding this comment

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

In the coming weeks, we will release: https://github.com/sanic-org/sanic-test

It will automatically set that.

@ahopkins ahopkins merged commit 8f79086 into ahopkins:master Jan 5, 2021
@ahopkins
Copy link
Owner

ahopkins commented Jan 5, 2021

Released. Thank you 👏

https://pypi.org/project/sanic-jwt/1.6.0/

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

3 participants