fix: support github token for plugin release checks#6886
fix: support github token for plugin release checks#68861zzxy1 wants to merge 2 commits intoAstrBotDevs:masterfrom
Conversation
dosubot
bot
added
the
size:L
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the plugin release check mechanism by introducing support for a configurable GitHub token. This allows the application to make authenticated requests to the GitHub API, thereby mitigating issues related to rate limiting that unauthenticated requests might encounter. The change integrates the token into the configuration, passes it through the update process, and ensures it's used securely and selectively for GitHub-specific API calls. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
dosubot
bot
added
the
feature:updater
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
RepoZipUpdator._build_request_headers, consider returningNoneinstead of an empty dict when no auth header is needed so that callers can avoid passingheaders={}toaiohttp.ClientSession, which overwrites default headers unnecessarily. - The new
github_tokenconfiguration is only read from the top-level config inPluginManager; if other consumers ofRepoZipUpdatoror plugin-related code should also respect this token, it may be worth centralizing how the token is retrieved (e.g., via a helper) to avoid divergence between call sites.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `RepoZipUpdator._build_request_headers`, consider returning `None` instead of an empty dict when no auth header is needed so that callers can avoid passing `headers={}` to `aiohttp.ClientSession`, which overwrites default headers unnecessarily.
- The new `github_token` configuration is only read from the top-level config in `PluginManager`; if other consumers of `RepoZipUpdator` or plugin-related code should also respect this token, it may be worth centralizing how the token is retrieved (e.g., via a helper) to avoid divergence between call sites.
## Individual Comments
### Comment 1
<location path="tests/test_plugin_manager.py" line_range="81-90" />
<code_context>
return mock_reload
+def test_plugin_manager_passes_github_token_to_updator(monkeypatch):
+ captured = {}
+
+ class DummyUpdator:
+ def __init__(self, repo_mirror: str = "", github_token: str = ""):
+ captured["repo_mirror"] = repo_mirror
+ captured["github_token"] = github_token
+
+ class MockContext:
+ def get_all_stars(self):
+ return []
+
+ monkeypatch.setattr("astrbot.core.star.star_manager.PluginUpdator", DummyUpdator)
+ monkeypatch.setattr(
+ "astrbot.core.star.star_tools.StarTools.initialize",
+ lambda context: None,
+ )
+
+ pm = PluginManager(
+ cast(Any, MockContext()),
+ cast(Any, {"github_token": "ghp_test"}),
+ )
+
+ assert isinstance(pm.updator, DummyUpdator)
+ assert captured == {"repo_mirror": "", "github_token": "ghp_test"}
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a complementary test for the case where `github_token` is not present or empty in the config.
Please also add a test where `github_token` is missing from the config or set to an empty string. That test should confirm that `PluginUpdator` is still constructed and that `github_token` is passed as an empty value, documenting the behavior when no token is provided and guarding against future regressions in `PluginManager` config handling.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def test_plugin_manager_passes_github_token_to_updator(monkeypatch): | ||
| captured = {} | ||
|
|
||
| class DummyUpdator: | ||
| def __init__(self, repo_mirror: str = "", github_token: str = ""): | ||
| captured["repo_mirror"] = repo_mirror | ||
| captured["github_token"] = github_token | ||
|
|
||
| class MockContext: | ||
| def get_all_stars(self): |
There was a problem hiding this comment.
suggestion (testing): Add a complementary test for the case where github_token is not present or empty in the config.
Please also add a test where github_token is missing from the config or set to an empty string. That test should confirm that PluginUpdator is still constructed and that github_token is passed as an empty value, documenting the behavior when no token is provided and guarding against future regressions in PluginManager config handling.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively adds support for a GitHub token to authenticate API requests for plugin release checks, which is a great way to avoid rate-limiting issues. The implementation correctly restricts token usage to api.github.com requests and includes solid testing to verify the new functionality. I have a couple of suggestions to further improve the changes.
astrbot/core/zip_updator.py
Outdated
| headers: dict[str, str] = {} | ||
| parsed_url = urlparse(url) | ||
| if self.github_token and parsed_url.netloc == "api.github.com": | ||
| headers["Authorization"] = f"token {self.github_token}" |
There was a problem hiding this comment.
While using token {self.github_token} for the Authorization header is valid, GitHub's documentation recommends using the Bearer scheme for authenticating with personal access tokens. It's a more modern and standard approach.
| headers["Authorization"] = f"token {self.github_token}" | |
| headers["Authorization"] = f"Bearer {self.github_token}" |
| }, | ||
| "github_token": { | ||
| "description": "GitHub Token", | ||
| "hint": "Optional. Used for GitHub Releases API requests to avoid unauthenticated rate limit exceeded errors. A read-only token is recommended." |
There was a problem hiding this comment.
The hint for github_token is in English. For consistency within this localization file, it should be translated into Russian.
| "hint": "Optional. Used for GitHub Releases API requests to avoid unauthenticated rate limit exceeded errors. A read-only token is recommended." | |
| "hint": "Необязательно. Используется для запросов к GitHub Releases API, чтобы избежать превышения лимита неаутентифицированных запросов. Рекомендуется использовать токен только для чтения." |
1 participant
Summary
Closes #6870
Testing
Summary by Sourcery
Add configurable GitHub token support for plugin release checks and ensure it is correctly propagated and used only for GitHub API requests.
New Features:
Enhancements:
Documentation:
Tests: