added methods for asset info and payout rates#62
added methods for asset info and payout rates#62theshadow76 merged 13 commits intoChipaDevTeam:mainfrom
Conversation
Summary of ChangesHello @NellyAngelz, 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 significantly upgrades the PocketOption API client's ability to manage and expose asset and payout data. It introduces dedicated methods for accessing asset details and payout rates, backed by a more sophisticated message parsing mechanism for incoming WebSocket data. The changes ensure that the client can efficiently store and provide up-to-date information on various assets, including their tradability and OTC status, enhancing the overall utility and responsiveness of the API. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds structured payout parsing in the websocket client, emits a consolidated Changes
Sequence DiagramsequenceDiagram
participant WebSocket as WebSocket Server
participant WSClient as AsyncWebSocketClient
participant EventSystem as Event System
participant Client as AsyncPocketOptionClient
participant App as Application
WebSocket->>WSClient: Send payout message (bytes)
WSClient->>WSClient: Decode bytes -> JSON
WSClient->>WSClient: Detect payout message ([[5,...]])
WSClient->>WSClient: _handle_payout_message(json)
WSClient->>WSClient: Parse assets -> finalData
WSClient->>EventSystem: emit('payout_update', finalData)
EventSystem->>Client: _on_payout_update(finalData)
Client->>Client: Update _payout_cache and _asset_info
App->>Client: get_payout(asset) / get_asset_info(asset)
Client->>App: Return cached value
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pocketoptionapi_async/client.py (1)
289-319:⚠️ Potential issue | 🟠 Major
payout_updateevent not wired to the keep-alive manager — payout cache is never populated for persistent connections
_setup_event_handlersregisterspayout_update → _on_payout_updateonly onself._websocket. Whenpersistent_connection=True, all WebSocket events are routed through_keep_alive_manager, notself._websocket. Becausepayout_updateis never registered on the keep-alive manager (compare the other handlers added at lines 289–318),_on_payout_updateis never called,_payout_cacheand_asset_infostay empty for the lifetime of the connection, andget_payout/get_asset_infoalways returnNone.🐛 Proposed fix
self._keep_alive_manager.add_event_handler("json_data", self._on_json_data) + self._keep_alive_manager.add_event_handler( + "payout_update", self._on_payout_update + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pocketoptionapi_async/client.py` around lines 289 - 319, _setup_event_handlers currently registers the "payout_update" → _on_payout_update handler only on self._websocket, so when persistent_connection uses self._keep_alive_manager the payout_update event is never handled and _payout_cache/_asset_info remain empty; add a matching registration on self._keep_alive_manager (i.e., call self._keep_alive_manager.add_event_handler("payout_update", self._on_payout_update)) alongside the other keep-alive registrations so _on_payout_update is invoked for forwarded events.pocketoptionapi_async/websocket_client.py (1)
607-636:⚠️ Potential issue | 🟠 Major
_process_message_optimizedpasses a raw string to_handle_payout_message, which now expects pre-parsed JSONIn
_process_message(line 515) the handler is called withjson_data— the already-parsed Python list. In_process_message_optimized, however, bytes are only decoded to a string (line 616–617); the prefix router then callsawait handler(message)wheremessageis still the raw"[[5,..."string. Inside_handle_payout_message,raw_assets = json_messageassigns that string, andfor asset in raw_assets:iterates over individual characters instead of asset arrays. Every innertryblock silently continues, resulting in zero assets being parsed with no error logged.Either parse the JSON before dispatching in
_process_message_optimized, or make_handle_payout_messageaccept both forms:🐛 Proposed fix (parse before dispatch in _process_message_optimized)
async def _process_message_optimized(self, message) -> None: try: if isinstance(message, bytes): message = message.decode("utf-8") + # Pre-parse payout messages so _handle_payout_message receives a list + if message.startswith("[[5,"): + try: + json_data = json.loads(message) + await self._handle_payout_message(json_data) + except Exception as e: + logger.error(f"Payout message parsing error: {e}") + return + logger.debug(f"Received message: {message}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pocketoptionapi_async/websocket_client.py` around lines 607 - 636, The optimized processor currently passes a raw decoded string to handlers causing _handle_payout_message (which expects parsed JSON) to iterate characters; modify _process_message_optimized to parse the decoded message into Python objects before dispatch: after decoding bytes to str, call json.loads(message) (handle JSONDecodeError similarly to _process_message), update caching keys to use the original string but pass the parsed json_data into handlers from self._message_handlers, and keep the cached_result behavior consistent (store/retrieve parsed objects). Ensure exceptions are caught and logged the same way as in _process_message so _handle_payout_message receives a list/dict not a raw string.
🧹 Nitpick comments (3)
pocketoptionapi_async/client.py (2)
160-161:get_payoutis missing a docstringAll other public methods on
AsyncPocketOptionClienthave docstrings. For consistency and API clarity, add one here.♻️ Proposed fix
def get_payout(self, asset: str) -> Optional[float]: + """Get cached payout rate for the given asset symbol, or None if unavailable.""" return self._payout_cache.get(asset)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pocketoptionapi_async/client.py` around lines 160 - 161, Add a docstring to the public method get_payout on AsyncPocketOptionClient explaining its purpose (returns the cached payout for a given asset), the parameter (asset: str) and the return type (Optional[float] — payout value or None if not cached), and any important behavior (uses internal _payout_cache, does not fetch remote data). Place the docstring immediately under the def get_payout(...) signature following the style used by other public methods in the class.
135-147:_on_payout_updateis synchronous while every other registered handler isasyncAll other handlers (
_on_authenticated,_on_balance_updated, etc.) areasync def. This one is a plaindef. The event dispatcher handles both, so it is not a functional bug, but the inconsistency makes the code harder to maintain and is easy to break if logging or cache updates ever need toawaitsomething.♻️ Proposed fix
- def _on_payout_update(self, data) -> None: + async def _on_payout_update(self, data) -> None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pocketoptionapi_async/client.py` around lines 135 - 147, The _on_payout_update handler is currently a synchronous def while all other event handlers are async; change its definition to async def _on_payout_update(self, data) -> None and keep the same body (await nothing for now) so it matches the other handlers' coroutine signature; verify any registrations that reference _on_payout_update (event dispatcher) continue to work (the dispatcher already supports async handlers) and update its type annotation if your linter requires an explicit Coroutine return type.pocketoptionapi_async/websocket_client.py (1)
248-250: Silentexcept Exception: continuediscards individual asset parse errors without any diagnosticWhen a single asset entry is malformed (e.g. missing index 15 or a wrong type), the failure is swallowed with no debug output, making it impossible to identify bad server payloads. Consider at least a
logger.debugas Ruff S112 also recommends.♻️ Proposed fix
except Exception: - # Skip broken entries safely - continue + logger.debug(f"Skipping malformed asset entry: {asset!r}") + continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pocketoptionapi_async/websocket_client.py` around lines 248 - 250, The silent "except Exception: continue" in websocket_client.py swallows parse errors for individual asset entries; modify the handler around that except block (the one catching exceptions while iterating asset entries) to log the exception and offending data instead of silently continuing — e.g., replace the bare except with a logger.debug or logger.exception call that includes exc_info and the asset/raw entry content (so you still continue on error but have diagnostics to identify malformed payloads).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pocketoptionapi_async/client.py`:
- Around line 96-97: Update the _payout_cache annotation to allow None values
because payout = info.get("payout") can be None and is stored directly; change
its type from Dict[str, float] to Dict[str, Optional[float]] (or
Mapping/MutableMapping with Optional[float]) and import Optional from typing if
not already present; ensure this matches the get_payout return type and any
consumers of _payout_cache (references to _payout_cache, payout, and get_payout)
remain type-consistent.
In `@pocketoptionapi_async/websocket_client.py`:
- Around line 269-271: The except block in AsyncWebSocketClient uses
self.enable_logging which doesn't exist on that class, causing an AttributeError
while handling another exception; update the handler around the payout parsing
(the except Exception as e: that currently does if self.enable_logging:
logger.error(...)) to log unconditionally via logger.error(f"Payout message
parsing error: {e}") (or guard with getattr(self, 'enable_logging', True) if you
prefer), so the original parsing error is properly reported; target the except
block in websocket_client.py where the logger.error call is made for payout
parsing errors.
- Around line 253-265: finalData is a plain dict so attribute assignment like
finalData.assets = parsed_assets raises AttributeError; replace the four
attribute-style assignments in the payout update block (references: finalData,
parsed_assets, keys "assets", "otc_assets", "real_assets", "tradable_assets")
with dict key assignments using bracket notation (e.g. finalData["assets"] =
parsed_assets) so the values are stored and the payout_update event emits the
populated data.
---
Outside diff comments:
In `@pocketoptionapi_async/client.py`:
- Around line 289-319: _setup_event_handlers currently registers the
"payout_update" → _on_payout_update handler only on self._websocket, so when
persistent_connection uses self._keep_alive_manager the payout_update event is
never handled and _payout_cache/_asset_info remain empty; add a matching
registration on self._keep_alive_manager (i.e., call
self._keep_alive_manager.add_event_handler("payout_update",
self._on_payout_update)) alongside the other keep-alive registrations so
_on_payout_update is invoked for forwarded events.
In `@pocketoptionapi_async/websocket_client.py`:
- Around line 607-636: The optimized processor currently passes a raw decoded
string to handlers causing _handle_payout_message (which expects parsed JSON) to
iterate characters; modify _process_message_optimized to parse the decoded
message into Python objects before dispatch: after decoding bytes to str, call
json.loads(message) (handle JSONDecodeError similarly to _process_message),
update caching keys to use the original string but pass the parsed json_data
into handlers from self._message_handlers, and keep the cached_result behavior
consistent (store/retrieve parsed objects). Ensure exceptions are caught and
logged the same way as in _process_message so _handle_payout_message receives a
list/dict not a raw string.
---
Nitpick comments:
In `@pocketoptionapi_async/client.py`:
- Around line 160-161: Add a docstring to the public method get_payout on
AsyncPocketOptionClient explaining its purpose (returns the cached payout for a
given asset), the parameter (asset: str) and the return type (Optional[float] —
payout value or None if not cached), and any important behavior (uses internal
_payout_cache, does not fetch remote data). Place the docstring immediately
under the def get_payout(...) signature following the style used by other public
methods in the class.
- Around line 135-147: The _on_payout_update handler is currently a synchronous
def while all other event handlers are async; change its definition to async def
_on_payout_update(self, data) -> None and keep the same body (await nothing for
now) so it matches the other handlers' coroutine signature; verify any
registrations that reference _on_payout_update (event dispatcher) continue to
work (the dispatcher already supports async handlers) and update its type
annotation if your linter requires an explicit Coroutine return type.
In `@pocketoptionapi_async/websocket_client.py`:
- Around line 248-250: The silent "except Exception: continue" in
websocket_client.py swallows parse errors for individual asset entries; modify
the handler around that except block (the one catching exceptions while
iterating asset entries) to log the exception and offending data instead of
silently continuing — e.g., replace the bare except with a logger.debug or
logger.exception call that includes exc_info and the asset/raw entry content (so
you still continue on error but have diagnostics to identify malformed
payloads).
![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 introduces functionality for handling asset payout information, including caching and new getter methods. The refactoring of the payout message handler is a significant improvement in terms of readability and robustness. I've identified a critical bug in the new message handler that would cause a runtime error, along with some incorrect type hints and opportunities for improving code clarity and robustness. Addressing these points will further enhance the quality of the implementation.
| self._server_time: Optional[ServerTime] = None | ||
| self._event_callbacks: Dict[str, List[Callable]] = defaultdict(list) | ||
| self._payout_cache: Dict[str, float] = {} # Cache for payout data | ||
| self._asset_info: Dict[str, Any] = {} # Store asset information from payout messages |
There was a problem hiding this comment.
Using Any for _asset_info reduces type safety and makes the code harder to maintain. Consider defining a TypedDict or a Pydantic BaseModel for the asset information structure. This will make the code more robust, self-documenting, and provide better support for static analysis and editor autocompletion.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Removed redundant logging condition checks in asset data parsing and payout message error handling.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pocketoptionapi_async/client.py (1)
972-983:⚠️ Potential issue | 🟠 Major
countandend_timeare currently ignored in candle requests.Line 972 accepts
countandend_time, and callers pass both (Line 485), but the payload at Line 976-983 only sendsassetandperiod. This silently drops user intent and makesget_candles(..., count=..., end_time=...)misleading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pocketoptionapi_async/client.py` around lines 972 - 983, The _request_candles function is dropping the caller-supplied count and end_time (so get_candles is misleading); update the payload built in variable data inside _request_candles to include the requested count and a machine-friendly end timestamp (e.g., add "count": count and a field like "to" or "endTime" set to int(end_time.timestamp()) or equivalent expected by PocketOption), then keep building message_data and message as before; ensure you convert end_time to an integer/epoch value and preserve asset and period keys so callers' count and end_time are actually sent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pocketoptionapi_async/client.py`:
- Line 384: The staleness check uses (datetime.now() -
self._balance.last_updated).seconds which ignores days; change the comparison to
use total seconds by calling (datetime.now() -
self._balance.last_updated).total_seconds() > 60 so that the full timedelta is
considered; update the conditional in the code path that references
self._balance and last_updated (the balance refresh check) to use
total_seconds() instead of .seconds.
- Around line 670-672: The error message block uses multiple f-strings that
contain no placeholders (e.g., the lines composing the authentication hint that
include the literal '42["auth",{"session":"your_session_id",...}]'), which
triggers Ruff F541; remove the unnecessary f-prefix from all those string
literals (not just the lines noted in the comment) so they are normal strings
(or combine them into one non-f-string), updating the message construction in
the client.py error message block where the auth hint is built.
In `@pocketoptionapi_async/websocket_client.py`:
- Around line 500-509: The string-path in AsyncWebSocketClient._process_message
currently skips the payout-frame detection added to the bytes-path; replicate
the same check from _process_message_optimized inside _process_message: after
decoding/parsing the JSON string, detect if json_data is a list whose first
element is a list starting with 5 and length>1, and if so call await
self._handle_payout_message(json_data) and return. Keep the condition identical
to the bytes-path and use the same helper method name (_handle_payout_message)
so both processing paths behave consistently.
---
Outside diff comments:
In `@pocketoptionapi_async/client.py`:
- Around line 972-983: The _request_candles function is dropping the
caller-supplied count and end_time (so get_candles is misleading); update the
payload built in variable data inside _request_candles to include the requested
count and a machine-friendly end timestamp (e.g., add "count": count and a field
like "to" or "endTime" set to int(end_time.timestamp()) or equivalent expected
by PocketOption), then keep building message_data and message as before; ensure
you convert end_time to an integer/epoch value and preserve asset and period
keys so callers' count and end_time are actually sent.
|
the pull should be ready for release now👍 |
2 participants
completed the left out payout implementation and added a decoding and formatting for the [[5, messages! please review and publish a newer version of pocketoptionapi_async to pip<3
Summary by CodeRabbit