-
Notifications
You must be signed in to change notification settings - Fork 62.6k
fix python example in generating-a-json-web-token-jwt-for-a-github-app #39409
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
Conversation
Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the Python code example in the GitHub App JWT generation documentation to use the correct PyJWT library API. The changes update the outdated jwt
module usage to the modern JWT
class-based approach with proper key handling.
Key changes:
- Updates import statement to use the modern JWT class and jwk_from_pem function
- Modifies key loading to use jwk_from_pem for proper PEM file handling
- Changes JWT encoding to use the instance-based approach with updated parameter names
@@ -115,7 +115,8 @@ payload = { | |||
} | |||
|
|||
# Create JWT | |||
encoded_jwt = jwt.encode(payload, signing_key, algorithm='RS256') | |||
instance = JWT() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter name change from 'algorithm' to 'alg' should be documented or explained in the surrounding text to help users understand why this change was made, especially since this differs from the previous API.
instance = JWT() | |
instance = JWT() | |
# The 'alg' parameter specifies the algorithm used for signing the JWT. | |
# It corresponds to the 'algorithm' parameter in the previous API. |
Copilot uses AI. Check for mistakes.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
How to review these changes 👓Thank you for your contribution. To review these changes, choose one of the following options: A Hubber will need to deploy your changes internally to review. Table of review linksNote: Please update the URL for your staging server or codespace. The table shows the files in the
Key: fpt: Free, Pro, Team; ghec: GitHub Enterprise Cloud; ghes: GitHub Enterprise Server 🤖 This comment is automatically generated. |
@gregsadetsky Thanks for opening an issue and PR! I'll get this triaged for review. |
@gregsadetsky My co-worker tells me this is essentially a reversion to an earlier state, but agrees it's not a bad move currently. The only thing is that on line 79, the |
@Sharra-writes oh my God, this is super embarrassing ... I actually didn't fully follow the original tutorial and used a different jwt library. The library that you're using seems a lot more standard. And so, the example in the docs wasn't wrong at all... I'm really sorry! I'll just close this PR then, yeah? |
@gregsadetsky It's no problem! I can close it for you. Thanks for reaching out to improve things, even if it went a bit sideways this time. We still appreciate the effort and concern. |
A good slice of humble pie for me! Haha. Cheers & thank you |
Why:
Closes: #39408
What's being changed (if available, include any code snippets, screenshots, or gifs):
the python example has been fixed
Check off the following: