Swap calls to extractall for TarSafe equivalent - 2.8.x #9851
Conversation
poetry.lock
Outdated
| @@ -1849,18 +1849,18 @@ python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" | |||
|
|
|||
| [[package]] | |||
| name = "pyjwt" | |||
| version = "2.1.0" | |||
| version = "2.2.0" | |||
There was a problem hiding this comment.
There are quite a few packages being updated - I am not very familiar with poetry, why would this be caused by adding the tarsafe requirement? Perhaps this could also be the reason why a test in test_server.py was failing at first with an error related to JWT?
There was a problem hiding this comment.
That could be - I added the package with poetry add tarsafe and it went around updating things (I thought within the version specifications, but maybe not). I'll have a look into this
There was a problem hiding this comment.
Yeah, it looks like the pyproject.toml specifies what versions we accept and it may be that there's some breaking change in a later version that is accepted
There was a problem hiding this comment.
Is there a way to revert pyjwt to version 2.1.0 then since fixing this is out of scope of this PR?
There was a problem hiding this comment.
I can probably alter the lock file but that's not a good idea generally. It looks like we don't use it as an immediate dependency so it must be a transitive dependency, and whatever is pulling it in isn't specifying what version it wants clearly enough maybe?
There was a problem hiding this comment.
Yes - it looks like oauthlib is pulling it in, and it's specifying "pyjwt (>=2.0.0,<3)", and sanic-jwt is pulling it in specifying *. The latest release of pyjwt is 2.2.0 and that fits the constraints so that's been updated, but I think the constraint set by sanic-jwt doesn't take into account the changes made in it. It looks like on the sanic-jwt repo this was fixed 2 months ago, but to pull that in we'd have to bump it's version to 1.7.0. Not sure what the best approach here is going to be, any ideas @wochinge? It looks as if the changes in 1.7.0 vs 1.5.0 (what we have now) are fairly minor, so I could try bumping it on this PR and if the tests pass we go ahead with it?
There was a problem hiding this comment.
Mhm, weird that poetry add tarsafe caused this. My suggested approach would be to limit the upper bound for now via pyproject.toml and then create an issue to bump this + fix any related issues. This avoids mixing the scopes (especially since this will also effect Rasa Enterprise which makes quite heavy use of sanic-jwt)
Real fix is to update sanic-jwt (and then drop the pin) but we are concerned about breakage at the moment so this is an easy temporary fix
Proposed changes:
main)Status (please check what you already did):
black(please check Readme for instructions)