added pyrogram support#5
Open
chizumeiji wants to merge 3 commits into
Open
Conversation
|
Warning Review limit reached
More reviews will be available in 39 minutes and 17 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces a new ChangesPyrogram Session Format and Conversion
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/pg/client.py (1)
275-301: 💤 Low valueUse explicit
is not Nonecheck for consistency.Line 294 uses
api or API.TelegramDesktop, whereas line 210 inFromTDesktopusesapi if api is not None else .... While functionally safe forAPIDatatypes, the inconsistency reduces readability. Useis not Noneconsistently across the class.♻️ Proposed fix for consistency
- use_api = api or API.TelegramDesktop + use_api = api if api is not None else API.TelegramDesktop() client = await self.ToTelethon(api=use_api)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pg/client.py` around lines 275 - 301, The code in ToTDesktop uses a truthy check "use_api = api or API.TelegramDesktop" which is inconsistent with FromTDesktop's explicit None check; change the assignment in ToTDesktop to "use_api = api if api is not None else API.TelegramDesktop" (keeping the rest of the method—calls to ToTelethon and TDesktop.FromTelethon—unchanged) so the API selection is done via an explicit is not None check for consistency with the class.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pg/client.py`:
- Around line 106-129: The constructor __init__ currently assigns self.auth_key
(and other attrs) before running the Expects validation, leaving a
partially-initialized instance on failure; move the auth_key validation (the
Expects(...) call that raises SessionFileInvalid when auth_key is not 256 bytes)
to the top of __init__ before any assignments, then convert/assign self.auth_key
= bytes(auth_key) if valid and proceed to set self.dc_id, self.api_id,
self.user_id, self.is_bot, and self.test_mode; keep the same exception type and
message and leave other logic unchanged.
- Around line 49-85: The code in _build_telethon_session silently falls back to
a hardcoded datacenter when dc_id is missing from DC_ADDRESSES; add explicit
validation at the start of the function to ensure dc_id is an expected value
(e.g., 1–5 or membership in DC_ADDRESSES) and fail fast: if dc_id not in
DC_ADDRESSES, raise a ValueError (or alternatively log.warning and then raise)
with a clear message including the invalid dc_id so callers don't silently
connect to the hardcoded address; update _build_telethon_session to perform this
check before using DC_ADDRESSES.
- Around line 25-46: In _resolve_api_from_id, remove the unreachable "return api
# type: ignore[return-value]" path and instead make the function explicitly
handle invalid `api` inputs: when `api is not None` and it's neither an instance
of APIData nor a subclass of APIData (the branches covering isinstance(api,
APIData) and issubclass(api, APIData)), raise a clear TypeError or ValueError
indicating an invalid `api` argument; keep the rest of the function (the
pyro_api_id checks and default return API.TelegramAndroid()) unchanged so
callers get a deterministic error rather than the unreachable implicit return.
- Around line 150-196: The FromTelethon method currently swallows all exceptions
from client.get_me() which hides failures; change the try/except around the
await client.get_me() to catch narrower Telethon errors (e.g.,
telethon.errors.RPCError or telethon.errors.TelethonError) or, if those types
are not imported here, catch Exception as e but log it instead of suppressing
it; add a module logger (logger = logging.getLogger(__name__)) and in the except
block call logger.warning or logger.exception with context like "FromTelethon:
failed to fetch user info via get_me()" and include the exception (exc_info=True
or pass e) so the function still falls back to existing behavior (leave user_id
as None) but the failure is recorded for debugging.
In `@src/pg/pyrogram_io.py`:
- Around line 167-185: The export code never passes the account type flag to
write_pyrogram_session, so bot exports are serialized as user sessions; modify
the logic around fetch_user_info / client.get_me and the fallback using
client.UserId to also determine whether the current account is a bot (e.g., set
is_bot = True/False by checking me.is_bot when me is present, or by reading the
appropriate attribute from the client when using client.UserId), then pass that
is_bot value into the write_pyrogram_session(...) call so the exported session
preserves bot vs user semantics.
- Around line 34-35: The helper builds full_path with "full_path = path if
path.endswith('.session') else path + '.session'" but may leave relative paths;
normalize it to the canonical absolute path by wrapping the result with
os.path.abspath (e.g. full_path = os.path.abspath(path if
path.endswith('.session') else path + '.session')) before opening or returning
the session file so the written location and the returned value are aligned;
apply the same change to the other identical occurrence of that assignment.
In `@src/pg/session_string.py`:
- Around line 30-35: For src/pg/session_string.py (lines 30-35): avoid
evaluating len(auth_key) when auth_key may not be bytes by splitting the
validation into a type check first and only measuring length afterward (or by
guarding the length computation when constructing SessionFileInvalid); ensure
Expects() is passed a pre-built SessionFileInvalid that uses a safe length value
(e.g., 0 or "unknown") when auth_key is not a bytes-like object. For
src/pg/pyrogram_io.py (lines 127-133): apply the identical fix in the SQLite
reader — separate the isinstance(auth_key, (bytes, bytearray)) test from the
length check or guard the len() call when formatting the SessionFileInvalid
message so malformed files raise SessionFileInvalid rather than triggering
TypeError.
---
Nitpick comments:
In `@src/pg/client.py`:
- Around line 275-301: The code in ToTDesktop uses a truthy check "use_api = api
or API.TelegramDesktop" which is inconsistent with FromTDesktop's explicit None
check; change the assignment in ToTDesktop to "use_api = api if api is not None
else API.TelegramDesktop" (keeping the rest of the method—calls to ToTelethon
and TDesktop.FromTelethon—unchanged) so the API selection is done via an
explicit is not None check for consistency with the class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aacf96a1-d02c-4ac2-a8c9-8d1e99773342
📒 Files selected for processing (9)
src/__init__.pysrc/pg/__init__.pysrc/pg/client.pysrc/pg/configs.pysrc/pg/pyrogram_io.pysrc/pg/session_string.pysrc/pg/shared.pysrc/td/tdesktop.pysrc/tl/telethon.py
Comment on lines
+34
to
+35
| full_path = path if path.endswith(".session") else path + ".session" | ||
|
|
There was a problem hiding this comment.
Return the canonical absolute path.
TelegramClient.ToPyrogram() documents an absolute path, but this helper returns a relative path unchanged when the caller passes one. Normalize full_path with os.path.abspath() before opening the database so the write location and return value stay aligned.
📌 Minimal fix
- full_path = path if path.endswith(".session") else path + ".session"
+ full_path = os.path.abspath(
+ path if path.endswith(".session") else path + ".session"
+ )Also applies to: 90-90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pg/pyrogram_io.py` around lines 34 - 35, The helper builds full_path with
"full_path = path if path.endswith('.session') else path + '.session'" but may
leave relative paths; normalize it to the canonical absolute path by wrapping
the result with os.path.abspath (e.g. full_path = os.path.abspath(path if
path.endswith('.session') else path + '.session')) before opening or returning
the session file so the written location and the returned value are aligned;
apply the same change to the other identical occurrence of that assignment.
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
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.
приветик
реализовал поддержку pyrogram, которой как по мне сильно не хватало проекту, протестировал на одном из своих аккаунтов сконвертировав сессию во все возможные форматы и обратно, и все работает отлично
буду благодарен если примешь пулл реквест, и обновишь либу
Summary by CodeRabbit