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 fine-grained tokens #208

Merged
merged 2 commits into from
Oct 7, 2023

Conversation

froggleston
Copy link
Contributor

Summary

This PR adds support for the beta fine-grained personal access tokens (PAT)

Changes implemented

  • Adds a new command line argument, token_fine with the new -f flag to support the new token format.
  • Modifies the existing token command line argument into token_classic, keeping the flag -t

Issues addressed

This PR addresses #205

froggleston and others added 2 commits March 22, 2023 14:53
Add flags and example for fine-grained tokens
Copy link

@srl295 srl295 left a comment

Choose a reason for hiding this comment

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

it works but can be better

-t TOKEN, --token TOKEN
-f TOKEN_FINE, --token-fine TOKEN
fine-grained personal access token
-t TOKEN_CLASSIC, --token-classic TOKEN
Copy link

Choose a reason for hiding this comment

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

Suggested change
-t TOKEN_CLASSIC, --token-classic TOKEN
-t TOKEN_CLASSIC, --token TOKEN

don't rename the old arg

Comment on lines +371 to +373
if args.token_classic.startswith(_path_specifier):
args.token_classic = open(args.token_classic[len(_path_specifier):],
'rt').readline().strip()
Copy link

Choose a reason for hiding this comment

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

need to support this ( file:// ) for fine grained tokens too. otherwise you have to include the PAT on the command line.

@josegonzalez
Copy link
Owner

Mind rebasing this and responding to the above feedback?

@Ondkloss
Copy link
Contributor

What would be needed for this to be merge-able, @josegonzalez? Evaluating continuing the contribution, so it can be included in code base.

Regarding the existing flags, my assumption would be that being backwards compatible (not altering existing) would be preferrable. I also see comments that address certain errors if the fine grained token does not give enough permissions. I'm not sure I would address this as a first step.

@josegonzalez josegonzalez merged commit 61275c6 into josegonzalez:master Oct 7, 2023
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.

4 participants