Test #44
Test #44
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the error scraping workflow’s GitHub token permissions, updates supported Python versions, and modifies several HTTP error source TSV files. Flow diagram for updated error scraping workflow runflowchart TD
A["Workflow trigger (schedule or push)"] --> B["Start job build (scrape-errors)"]
B --> C["Checkout repository using GITHUB_TOKEN"]
C --> D["Run error scraping steps (read TSV sources)"]
D --> E["Write updated error artifacts to repository"]
E --> F["Commit or update files using contents:write permission"]
F --> G["Job completes"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Signed-off-by: 5hojib <yesiamshojib@gmail.com>
Summary of ChangesHello @5hojib, 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 primarily expands the API's error reporting capabilities by introducing a wide array of new error codes across different HTTP status categories (400, 403, 500). These additions aim to provide more granular and informative feedback for issues related to bot interactions, business features, payment processes, and general API usage. Concurrently, the project's configuration has been updated to reflect support for the upcoming Python 3.14 release. Highlights
Ignored Files
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Double-check whether
contents: writeis actually required for thescrape-errorsjob; if the workflow only needs to read from the repo or use OIDC, consider usingcontents: reador keepingid-token: writeto minimize token scope. - Before advertising
Programming Language :: Python :: 3.14support inpyproject.toml, ensure that the project is tested and compatible with that version and that the trove classifier is officially available to avoid misleading metadata.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Double-check whether `contents: write` is actually required for the `scrape-errors` job; if the workflow only needs to read from the repo or use OIDC, consider using `contents: read` or keeping `id-token: write` to minimize token scope.
- Before advertising `Programming Language :: Python :: 3.14` support in `pyproject.toml`, ensure that the project is tested and compatible with that version and that the trove classifier is officially available to avoid misleading metadata.
## Individual Comments
### Comment 1
<location> `.github/workflows/error_scrape.yml:8` </location>
<code_context>
build:
permissions:
- id-token: write
+ contents: write
name: scrape-errors
runs-on: ubuntu-latest
</code_context>
<issue_to_address>
**🚨 issue (security):** Revisit whether `contents: write` is the minimal permission needed for this workflow.
This change trades OIDC access for broader repo write access, which may exceed this job’s needs. If it only requires reading repo data, prefer `contents: read`. If you still need OIDC (e.g., to assume a cloud role), consider keeping `id-token: write` and adding only the minimal extra permissions required.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| build: | ||
| permissions: | ||
| id-token: write | ||
| contents: write |
There was a problem hiding this comment.
🚨 issue (security): Revisit whether contents: write is the minimal permission needed for this workflow.
This change trades OIDC access for broader repo write access, which may exceed this job’s needs. If it only requires reading repo data, prefer contents: read. If you still need OIDC (e.g., to assume a cloud role), consider keeping id-token: write and adding only the minimal extra permissions required.
![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 updates the project by adding numerous new Telegram API error definitions and declaring support for Python 3.14. The changes to the error source files appear to be part of an effort to normalize and keep them up-to-date with the official API documentation.
I've found one critical issue related to the addition of a new error ID that conflicts with an existing one, which will lead to a name collision in the generated Python exception classes. Please see my detailed comment on this.
Apart from this issue, the changes look good and are a valuable update to the library's error handling capabilities.
| NOT_ALLOWED Not allowed | ||
| NOT_ELIGIBLE You are not eligible for this action | ||
| PARTICIPANT_JOIN_MISSING Trying to enable a presentation, when the user hasn't joined the Video Chat with phone.joinGroupCall | ||
| PEER_ID_INVALID The provided peer id is invalid. |
There was a problem hiding this comment.
Adding the PEER_ID_INVALID error ID here introduces a name collision with an existing error. This ID is already defined in compiler/errors/source/400_BAD_REQUEST.tsv.
The error compiler (compiler/errors/compiler.py) generates Python exception class names by converting the error ID to CamelCase. In this case, both PEER_ID_INVALID entries will be converted to a class named PeerIdInvalid.
During the build process, the __init__.py file in the exceptions directory imports all classes from the generated modules (e.g., from .bad_request_400 import *, from .forbidden_403 import *). This will cause one of the PeerIdInvalid class definitions to overwrite the other, depending on the import order. This can lead to incorrect exception types being raised at runtime. For example, an error with code 403 might be raised as a subclass of BadRequest (400).
To resolve this, you should either ensure that error IDs are unique across all TSV files or modify the compiler to generate unique class names (e.g., by prefixing them with the error type like ForbiddenPeerIdInvalid). Since the error IDs seem to come directly from the Telegram API, modifying the compiler is likely the correct approach to handle these collisions systematically.
Summary by Sourcery
Adjust GitHub workflow permissions and update project metadata for a new Python version.
Build:
Chores: