Test #50
Merged
Test #50
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: 5hojib <yesiamshojib@gmail.com>
- Pin sphinx < 8.0.0 to fix compatibility issues with sphinx-immaterial. - Update CI workflows and build scripts to use uv for dependency management. - Fix docstring indentation errors in multiple files. - Clean up docs/source/conf.py by removing non-existent paths. - Update .gitignore to exclude build artifacts and generated RST files. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
* Make the library completely async and remove sync functions - Deleted `pyrogram/sync.py` which handled automatic sync wrapping. - Updated `pyrogram/__init__.py` to export `compose` and `idle` as async. - Removed synchronous context managers (`__enter__`, `__exit__`) from `Client`. - Removed `run_sync` utility from `Methods` and `utils`. - Simplified `app.run()` to handle async methods only. - Deleted synchronous usage documentation and updated references. Co-authored-by: 5hojib <107526130+5hojib@users.noreply.github.com> * Update docs/source/topics/scheduling.rst Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Contributor
Reviewer's GuideMigrates documentation and CI tooling to use uv-based workflows, cleans up deprecated synchronous utilities in the client API, tightens Sphinx/docs configuration and dependencies, and fixes/extends several type-level and parsing details in pyrogram internals. Sequence diagram for updated Client.run behavior without synchronous pathsequenceDiagram
actor User
participant App as ApplicationMain
participant Client
participant RunUtility as RunHelper
User->>App: invoke_client_run(optional_coroutine)
alt coroutine_provided
App->>RunUtility: run(coroutine)
RunUtility-->>App: coroutine_completed
else no_coroutine_provided
App->>RunUtility: run(Client.start())
RunUtility-->>Client: start()
Client-->>RunUtility: started
App->>RunUtility: run(idle())
RunUtility-->>RunUtility: wait_for_signals
App->>RunUtility: run(Client.stop())
RunUtility-->>Client: stop()
Client-->>RunUtility: stopped
end
Class diagram for updated Client utilities and EmojiStatus parsingclassDiagram
class Client {
+loop
+__init__(...)
+__aenter__() Client
+__aexit__(*args) None
+start() Client
+stop() None
}
class Utilities {
<<mixin>>
+Run
+Start
+Stop
+Restart
+RemoveHandler
+StopTransmissionError
+MethodUnavailableError
}
class EmojiStatus {
+client
+custom_emoji_id int
+until_date datetime
+_raw raw_base_EmojiStatus
+__init__(client, custom_emoji_id, until_date, _raw)
+_parse(client, emoji_status) EmojiStatus
}
class RawBaseEmojiStatus {
}
class RawEmojiStatusCollectible {
+document_id int
+until int
}
class Utils {
+timestamp_to_datetime(timestamp) datetime
}
Client ..|> Utilities
EmojiStatus --> RawBaseEmojiStatus : wraps
RawEmojiStatusCollectible --|> RawBaseEmojiStatus
EmojiStatus --> RawEmojiStatusCollectible : _raw
EmojiStatus --> Utils : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
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 focuses on a significant architectural shift by completely removing support for synchronous usage throughout the codebase and documentation. It also modernizes the project's documentation build process by integrating 'uv' for dependency management and execution, aiming for a more streamlined and consistent build environment. Additionally, the changes include minor code formatting improvements and an enhancement to the Highlights
Changelog
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}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The changes in
pyrogram/methods/utilities/run.pyremove the syncstart()path and now always treatself.startas async, which will break existing synchronous usage ofClient.run(); consider keeping a branch for non-coroutinestart()or clearly migrating callers before this change. - In
EmojiStatus._parse, the secondisinstancecheck was changed fromEmojiStatusUntiltoEmojiStatusCollectible, which looks like it drops support forEmojiStatusUntil; please double‑check whether both variants need to be handled and adjust the branching accordingly. - Removing
run_sync(and the sync context manager onClient) is an API change; if any internal or external callers still rely on those sync helpers, you may want to provide a compatibility shim or an explicit migration path to the async equivalents.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The changes in `pyrogram/methods/utilities/run.py` remove the sync `start()` path and now always treat `self.start` as async, which will break existing synchronous usage of `Client.run()`; consider keeping a branch for non-coroutine `start()` or clearly migrating callers before this change.
- In `EmojiStatus._parse`, the second `isinstance` check was changed from `EmojiStatusUntil` to `EmojiStatusCollectible`, which looks like it drops support for `EmojiStatusUntil`; please double‑check whether both variants need to be handled and adjust the branching accordingly.
- Removing `run_sync` (and the sync context manager on `Client`) is an API change; if any internal or external callers still rely on those sync helpers, you may want to provide a compatibility shim or an explicit migration path to the async equivalents.
## Individual Comments
### Comment 1
<location> `build-docs.sh:3-7` </location>
<code_context>
-VENV="$(pwd)/venv"
-export VENV
+
+uv sync --extra docs
make clean
</code_context>
<issue_to_address>
**suggestion:** Relying on `uv` in the build script can make local docs building fail if `uv` is not installed.
In CI, `uv` is provisioned by the workflow, but locally this script now assumes `uv` is installed and on PATH. Please add a check that emits a clear error if `uv` is missing, or fall back to the previous venv-based flow when `uv` is not available so local doc builds remain robust for contributors.
```suggestion
set -e
export GITHUB_TOKEN
if ! command -v uv >/dev/null 2>&1; then
echo "Error: 'uv' is required to build the documentation. Please install 'uv' or run this script from an environment where it is available on PATH." >&2
exit 1
fi
uv sync --extra docs
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
3
to
+7
| set -e | ||
|
|
||
| export GITHUB_TOKEN | ||
| VENV="$(pwd)/venv" | ||
| export VENV | ||
|
|
||
| uv sync --extra docs |
Contributor
There was a problem hiding this comment.
suggestion: Relying on uv in the build script can make local docs building fail if uv is not installed.
In CI, uv is provisioned by the workflow, but locally this script now assumes uv is installed and on PATH. Please add a check that emits a clear error if uv is missing, or fall back to the previous venv-based flow when uv is not available so local doc builds remain robust for contributors.
Suggested change
| set -e | |
| export GITHUB_TOKEN | |
| VENV="$(pwd)/venv" | |
| export VENV | |
| uv sync --extra docs | |
| set -e | |
| export GITHUB_TOKEN | |
| if ! command -v uv >/dev/null 2>&1; then | |
| echo "Error: 'uv' is required to build the documentation. Please install 'uv' or run this script from an environment where it is available on PATH." >&2 | |
| exit 1 | |
| fi | |
| uv sync --extra docs |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces significant and positive changes. The primary change is the removal of synchronous support, which simplifies the library's architecture and encourages the use of its asynchronous nature, which is the intended way. This breaking change is handled well, with corresponding updates to documentation, examples, and internal logic. The migration of the documentation build process and CI workflows to use uv is a great modernization step that simplifies dependency management and execution. Additionally, the adjustments to emoji status parsing to support collectible statuses and optional dates are valuable enhancements. The various minor formatting and docstring fixes across the codebase contribute to better readability and maintainability. Overall, this is a high-quality pull request with well-thought-out changes.
Signed-off-by: 5hojib <yesiamshojib@gmail.com>
Signed-off-by: 5hojib <yesiamshojib@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Update documentation build process to use uv-based workflows and simplify synchronous client utilities.
Enhancements:
Build:
CI:
Chores: