-
Notifications
You must be signed in to change notification settings - Fork 892
chore(utils): relax GitHub PAT validation (#380) #383
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
* Drop `InvalidGitHubTokenError` * `validate_github_token` now emits a `UserWarning` instead of raising * Update tests to: * assert no warnings for valid tokens (via `recwarn`) * expect `UserWarning` for malformed tokens * Reduces false-positives and is future-proof if GitHub introduces new token formats Closes #380
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.
We don't need to validate the token with pattern matching and the warning is useless here i guess.
When the token is invalid (malformed or just invalid) we correctly handle the error:
image
I don't see why we validate and log the validation result here.
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.
I think the validation is informative. Just a little message saying "this does not look like a proper token, tokens usually start with gh_ blablabla" because yeah, believe me, i've seen too many people struggle with access tokens and just put their plain password or something
This pull request has merge conflicts that must be resolved before it can be merged.
InvalidGitHubTokenErrorvalidate_github_tokennow emits aUserWarninginstead of raisingrecwarn)UserWarningfor malformed tokensCloses #380