Add error when unauthenticated in changelog tool #8855
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughRefines get_pr_number_from_gh in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/create-changelog-entry.py (1)
41-43: Nit: Print CLI-not-found message to stderr for consistency.For consistency with error conditions above and to keep stdout clean for normal output, print the "gh not found" message to stderr.
Outside the changed hunk, update the print to:
print("⚠️ GitHub CLI (`gh`) not found. You can install it from https://cli.github.com/", file=sys.stderr)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tools/create-changelog-entry.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
| except subprocess.CalledProcessError as e: | ||
| if "Bad credentials" in e.stderr: | ||
| print("⚠️ You are not authenticated with GitHub (Run 'gh auth login'). No pull request found.") | ||
| else: | ||
| print("⚠️ No pull request found for the current branch.") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Broaden auth error detection and print errors to stderr (more accurate guidance).
The GH CLI commonly reports unauth scenarios as "gh: Not logged in" or "HTTP 401: Unauthorized", not only "Bad credentials". Also, mixing an auth warning with "No pull request found" is confusing. Recommend:
- Case-insensitive matching against several common auth phrases.
- Fallback to e.output if stderr is empty.
- Print diagnostics to stderr.
Apply this diff:
-except subprocess.CalledProcessError as e:
- if "Bad credentials" in e.stderr:
- print("⚠️ You are not authenticated with GitHub (Run 'gh auth login'). No pull request found.")
- else:
- print("⚠️ No pull request found for the current branch.")
+except subprocess.CalledProcessError as e:
+ msg = (e.stderr or e.output or "").lower()
+ if any(s in msg for s in [
+ "bad credentials",
+ "not logged in",
+ "unauthorized",
+ "authentication failed",
+ "forbidden", # e.g., SAML enforcement/insufficient scopes
+ ]):
+ print("⚠️ You are not authenticated with GitHub. Run 'gh auth status' and 'gh auth login'.", file=sys.stderr)
+ else:
+ print("⚠️ No pull request found for the current branch.", file=sys.stderr)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except subprocess.CalledProcessError as e: | |
| if "Bad credentials" in e.stderr: | |
| print("⚠️ You are not authenticated with GitHub (Run 'gh auth login'). No pull request found.") | |
| else: | |
| print("⚠️ No pull request found for the current branch.") | |
| except subprocess.CalledProcessError as e: | |
| msg = (e.stderr or e.output or "").lower() | |
| if any(s in msg for s in [ | |
| "bad credentials", | |
| "not logged in", | |
| "unauthorized", | |
| "authentication failed", | |
| "forbidden", # e.g., SAML enforcement/insufficient scopes | |
| ]): | |
| print( | |
| "⚠️ You are not authenticated with GitHub. " | |
| "Run 'gh auth status' and 'gh auth login'.", | |
| file=sys.stderr, | |
| ) | |
| else: | |
| print( | |
| "⚠️ No pull request found for the current branch.", | |
| file=sys.stderr, | |
| ) |
I always had "No pull request found" and wanted a more useful error.