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

Detect Github access token when it is not OAuth token in cli.py #7102

Conversation

holisHsu
Copy link

@holisHsu holisHsu commented Apr 24, 2024

Background

Ref Issue: #7095

If user set a Github Fine-grained token, TypeError will be raised

CleanShot 2024-04-24 at 14 22 55@2x

And also because Fine grained token currently can not list permission, Github team is still working on it ..., we can not distinguish perms of Fine grained token and let setting procedure done.

In addition to above facts, I think we can consider to remind user this token should be an OAuth token when it's not, or user might confuse when they set a token but it turns out TypeError.

Changes πŸ—οΈ

In the setup process, if X-OAuth-Scopes is not exist in Github response, remind user to set an OAuth token.

CleanShot 2024-04-24 at 14 44 40@2x

Ref

Response header I get when using Github Fine-grained token

CleanShot 2024-04-23 at 01 41 47@2x

PR Quality Scorecard ✨

  • Have you used the PR description template?   +2 pts
  • Is your pull request atomic, focusing on a single change?   +5 pts
  • Have you linked the GitHub issue(s) that this PR addresses?   +5 pts
  • Have you documented your changes clearly and comprehensively?   +5 pts
  • Have you changed or added a feature?   -4 pts
    • Have you added/updated corresponding documentation?   +4 pts
    • Have you added/updated corresponding integration tests?   +5 pts
  • Have you changed the behavior of AutoGPT?   -5 pts
    • Have you also run agbenchmark to verify that these changes do not regress performance?   +10 pts

@CodiumAI-Agent
Copy link

Preparing PR description...

Copy link

netlify bot commented Apr 24, 2024

βœ… Deploy Preview for auto-gpt-docs ready!

Name Link
πŸ”¨ Latest commit 6750267
πŸ” Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/663c795b09037400087e101f
😎 Deploy Preview https://deploy-preview-7102--auto-gpt-docs.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@CodiumAI-Agent
Copy link

CodiumAI-Agent commented Apr 24, 2024

PR Review

(Review updated until commit a48560f)

⏱️ Estimated effort to review [1-5]

2, because the changes are localized to a single function in one file, and the logic added is straightforward. The PR is well-documented with background and screenshots, which aids in understanding the changes.

πŸ…Β Score

85

πŸ§ͺΒ Relevant tests

No

🎫 Relevant ticket

Yes

πŸ”Β Possible issues

Possible Bug: The variable install_error is set but not used elsewhere in the provided code snippet. This might indicate incomplete implementation or missing error handling logic.

πŸ”’Β Security concerns

No

πŸ”€Β Multiple PR themes

No


✨ Review tool usage guide:

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

  • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

cli.py Outdated Show resolved Hide resolved
cli.py Outdated Show resolved Hide resolved
cli.py Outdated Show resolved Hide resolved
@CodiumAI-Agent
Copy link

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

Preparing PR description...

@CodiumAI-Agent
Copy link

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

Persistent review updated to latest commit 72b8582

@holisHsu holisHsu force-pushed the adjust/github_token_not_oauth_token branch from 72b8582 to c84407c Compare April 24, 2024 06:53
@CodiumAI-Agent
Copy link

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

Preparing PR description...

@CodiumAI-Agent
Copy link

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

Persistent review updated to latest commit c84407c

@holisHsu holisHsu force-pushed the adjust/github_token_not_oauth_token branch from c84407c to a48560f Compare April 24, 2024 06:54
@CodiumAI-Agent
Copy link

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

Preparing PR description...

@CodiumAI-Agent
Copy link

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

Persistent review updated to latest commit a48560f

cli.py Outdated
install_error = True
click.echo(
click.style(
"❌ Response is lack of X-OAuth-Scopes, please make sure you use OAuth token.",
Copy link
Member

Choose a reason for hiding this comment

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

Lacking

Copy link
Author

Choose a reason for hiding this comment

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

Thank you, I have fixed it

Copy link
Author

@holisHsu holisHsu Apr 26, 2024

Choose a reason for hiding this comment

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

Comparing "is lacking in something" & "lack something"
I wonder maybe just "lack something" is clear enough

@holisHsu holisHsu requested a review from ntindle April 26, 2024 16:29
@holisHsu holisHsu force-pushed the adjust/github_token_not_oauth_token branch from c00e91e to 0468dfe Compare April 26, 2024 16:38
@holisHsu
Copy link
Author

holisHsu commented May 3, 2024

@ntindle Sorry to bother you
This PR have been here about 10 days
I am wondering if any maintainer want to give any advice or approval ?
Or I might consider to close it

@holisHsu
Copy link
Author

holisHsu commented May 4, 2024

Many thanks to ntindle.
Because there is some problem of CI, I am gonna just leave PR here :)

CleanShot 2024-05-04 at 23 36 34@2x
CleanShot 2024-05-04 at 23 37 06@2x

@ntindle
Copy link
Member

ntindle commented May 9, 2024

Good spot, this was actually supposed to be removed as a feature from the CLI so glad you caught it!

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label May 9, 2024
Copy link

github-actions bot commented May 9, 2024

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@ntindle
Copy link
Member

ntindle commented May 9, 2024

Closing as we are removing this functionality

@ntindle ntindle closed this May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts Automatically applied to PRs with merge conflicts size/m
Projects
Status: βœ… Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants