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

Work correctly with pyjwt 2.x #124

Merged
merged 2 commits into from
Dec 25, 2023
Merged

Work correctly with pyjwt 2.x #124

merged 2 commits into from
Dec 25, 2023

Conversation

athornton
Copy link
Collaborator

The actual type of jwt.encode() changed from bytes to str in 2.x, but somehow the typing hints aren't keeping up, and the whole JWT typing thing is a can of worms (cf jpadilla/pyjwt#602).

So I'm being overly defensive about what we actually got back from there, but returning a string up the call stack and modifying the function signature (and its caller).

This is the minimal necessary work to address #122

Maybe if I get some time in the New Year I can work on modernizing giftless and making its test suite more maintainable.

@athornton
Copy link
Collaborator Author

Test cases are passing now. Ready for review, I think.

@rufuspollock
Copy link
Member

@athornton great, thank-you. 👏 🙏

It looks good. Merging it in.

@rufuspollock rufuspollock merged commit f54c0ce into datopian:main Dec 25, 2023
4 checks passed
@gil-mahalla
Copy link

@rufuspollock
Where can I find the 0.5.1 container? It seems to be missing from docker hub, thank you!

@rufuspollock
Copy link
Member

@gil-mahalla we'd need to publish it - i think this is related to #119

@athornton
Copy link
Collaborator Author

@gil-mahalla if you've got the source checked out, and you've got Docker installed, docker build -t <your-org>/giftless:0.5.1 . should work, and then push it to your own repo. That's basically what I'm doing for now.

make docker might work for you, but it wants to use an existing image as build cache, which I think is going to get you outdated versions of the libraries. I had better luck just building the image from the Dockerfile in the repo. (You could also change the Makefile's DOCKER_REPO and DOCKER_CACHE_FROM; those are overriding variables so you can't force them from the command line, which is arguably a bug.)

@rufuspollock
Copy link
Member

@athornton if we could give you access to that repo on our docker hub would you up for pushing updates directly there - i imagine it would help your workflow (and that of others)?

@athornton athornton deleted the u/ajt/simpletypefix branch December 27, 2023 22:49
@athornton
Copy link
Collaborator Author

Sure. In the slightly longer term, we need the GitHub Action workflow to push on definition of a new release tag, I think, but in the interim, doing manual pushes of new artifacts would be OK with me. I'll be back at work January 2, 2024, and we can figure it out from there.

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