fixed some pylance errors and import errors #30
Conversation
WalkthroughThis update introduces type hint improvements, explicit error handling, and minor refactoring across multiple modules. Key changes include stricter type annotations for parameters and return values, enhanced safety checks for WebSocket operations, improved candle data parsing, and more robust handling of optional parameters. Several new public methods are added for connection management and statistics retrieval. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as AsyncPocketOptionClient
participant WS as WebSocketClientProtocol
participant KA as ConnectionKeepAlive
Client->>KA: connect_with_keep_alive(regions)
KA->>WS: Establish WebSocket connection
WS-->>KA: Connection established
KA-->>Client: Connection status (bool)
Client->>WS: Send/receive candle data
WS-->>Client: Candle data (with asset/timeframe)
Client->>Client: Parse and process candle data
Client->>KA: disconnect()
KA->>WS: Close connection
KA-->>Client: Disconnection complete
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
pocketoptionapi_async/websocket_client.py (1)
361-385: Consider refactoring message normalization logic.The message normalization logic is duplicated and could be extracted into a helper method for better maintainability.
+ def _normalize_message_to_string(self, message) -> str: + """Normalize message to string format""" + if isinstance(message, memoryview): + return bytes(message).decode("utf-8") + elif isinstance(message, (bytes, bytearray)): + return message.decode("utf-8") + return message + async def _send_handshake(self, ssid: str) -> None: """Send initial handshake messages (following old API pattern exactly)""" try: # ... existing code ... - # Ensure initial_message is a string - if isinstance(initial_message, memoryview): - initial_message = bytes(initial_message).decode("utf-8") - elif isinstance(initial_message, (bytes, bytearray)): - initial_message = initial_message.decode("utf-8") + initial_message = self._normalize_message_to_string(initial_message) # ... existing code ... - # Ensure conn_message is a string - if isinstance(conn_message, memoryview): - conn_message_str = bytes(conn_message).decode("utf-8") - elif isinstance(conn_message, (bytes, bytearray)): - conn_message_str = conn_message.decode("utf-8") - else: - conn_message_str = conn_message + conn_message_str = self._normalize_message_to_string(conn_message)pocketoptionapi_async/monitoring.py (2)
84-87: Good defensive programming with null check.The null check prevents potential comparison errors when
last_failure_timeis None. However, theelseclause afterraiseis unnecessary.Consider simplifying by removing the unnecessary
elseclause:- if ( - self.last_failure_time is not None - and time.time() - self.last_failure_time < self.recovery_timeout - ): - raise Exception("Circuit breaker is OPEN") - else: - self.state = "HALF_OPEN" + if ( + self.last_failure_time is not None + and time.time() - self.last_failure_time < self.recovery_timeout + ): + raise Exception("Circuit breaker is OPEN") + + self.state = "HALF_OPEN"
163-166: Excellent safeguard against missing exception data.This prevents a potential bug where
last_exceptioncould be None and we'd try to raise None. However, theelseclause afterraiseis unnecessary.Remove the unnecessary
elseclause afterraise:- if last_exception is not None: - raise last_exception - else: - raise Exception("RetryPolicy failed but no exception was captured.") + if last_exception is not None: + raise last_exception + + raise Exception("RetryPolicy failed but no exception was captured.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
pocketoptionapi_async/client.py(4 hunks)pocketoptionapi_async/connection_keep_alive.py(6 hunks)pocketoptionapi_async/connection_monitor.py(2 hunks)pocketoptionapi_async/constants.py(1 hunks)pocketoptionapi_async/exceptions.py(1 hunks)pocketoptionapi_async/monitoring.py(6 hunks)pocketoptionapi_async/websocket_client.py(11 hunks)tests/advanced_testing_suite.py(1 hunks)tests/integration_tests.py(1 hunks)tests/test_order_placement_fix.py(1 hunks)tools/client_test.py(2 hunks)tools/get_ssid.py(2 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
pocketoptionapi_async/monitoring.py
[refactor] 84-90: Unnecessary "else" after "raise", remove the "else" and de-indent the code inside it
(R1720)
[refactor] 163-166: Unnecessary "else" after "raise", remove the "else" and de-indent the code inside it
(R1720)
🔇 Additional comments (35)
pocketoptionapi_async/constants.py (1)
184-184: Good type hint improvement.The return type annotation
Optional[str]correctly reflects that the method may returnNonewhen the region is not found, improving type safety and code clarity.tools/get_ssid.py (2)
6-6: Excellent type import additions.Adding comprehensive type hints (
cast,List,Dict,Any) improves code clarity and enables better IDE support and static analysis.
98-103: Well-implemented runtime safety check.The defensive programming approach using
getattrandcallablecheck prevents runtime errors when the WebDriver doesn't supportget_log(). The clear error message guides users toward the correct WebDriver configuration.tools/client_test.py (3)
4-4: Correct import path update.The import change from
pocketoptionapi.constants.REGIONtopocketoptionapi_async.constants.REGIONSaligns with the async module refactoring and ensures compatibility with the updated constants structure.
10-12: Proper API usage update.Using
REGIONS.get_all()is the correct way to access region URLs with the new API structure, and the iteration over the returned list is handled correctly.
33-34: Improved type checking.Using
isinstance(message, bytes)is the proper Pythonic way to check type, more explicit and reliable than other type checking methods.pocketoptionapi_async/connection_keep_alive.py (6)
10-10: Improved import specificity.Importing specific components (
connect,WebSocketClientProtocol) fromwebsockets.legacy.clientprovides better type specificity and clearer dependency management.
26-26: Better type annotation.The specific
WebSocketClientProtocoltype annotation provides better type safety compared to a generic type, enabling better IDE support and static analysis.
141-141: Cleaner connection call.Using
connectdirectly after importing it specifically is cleaner and more explicit than using the full module path.
201-202: Essential safety check.Adding the websocket existence check before handshake operations prevents potential
AttributeErrorand provides a clear error message for debugging.
413-417: Proper defensive programming.The websocket existence check before sending pong responses prevents errors when the connection is in an unexpected state, improving robustness.
496-513: Well-designed public API methods.The three new methods (
connect_with_keep_alive,disconnect,get_stats) provide a clean, intuitive public interface for connection management and monitoring, with proper type hints and documentation.tests/advanced_testing_suite.py (1)
16-16: Good import path standardization.Changing from relative to absolute import path for
ConnectionKeepAlivemaintains consistency across the test suite and improves code clarity and maintainability.tests/test_order_placement_fix.py (1)
8-8: LGTM! Import organization improvement.Moving the import to the top of the file follows Python best practices and improves code readability.
tests/integration_tests.py (1)
15-17: LGTM! Import path improvements for better compatibility.The change from relative to absolute imports using full package paths improves import reliability and aligns with the package structure reorganization.
pocketoptionapi_async/connection_monitor.py (2)
605-605: LGTM! Default parameter clarification.Changing the default from
Noneto""maintains the same logic (both are falsy) while making the intent clearer.
726-726: LGTM! Type hint improvement.Adding
Optional[str]type hint improves code clarity and type safety.pocketoptionapi_async/websocket_client.py (7)
14-14: LGTM! Correct type import.Adding the specific
WebSocketClientProtocolimport improves type accuracy.
65-65: LGTM! Improved type annotations.Updating from generic
WebSocketServerProtocolto specificWebSocketClientProtocolprovides more accurate type information for client-side connections.Also applies to: 122-122
169-180: LGTM! Better connection handling pattern.Assigning the awaited connection to a local variable before setting the instance attribute is a cleaner pattern and improves type safety.
353-355: LGTM! Essential defensive check.Adding the WebSocket connection check before receiving messages prevents potential errors during handshake.
556-556: LGTM! Fixed cache key consistency.Converting the message hash to string before using as dictionary keys ensures consistent key types and prevents potential key lookup issues.
Also applies to: 571-575
672-673: LGTM! Simplified exception handling.Using bare
Exceptionis appropriate here for a fallback catch-all scenario.
681-682: LGTM! Essential null safety check.Adding the null check for
connection_infoprevents potential AttributeError when accessing the status property.pocketoptionapi_async/monitoring.py (5)
66-66: Good addition of Type import for better type annotations.This import enables more precise type hinting for the
expected_exceptionparameter.
72-72: Excellent type annotation improvement.Using
Type[BaseException]instead of justExceptionprovides better type safety and makes the intent clearer that this parameter expects an exception class, not an instance.
208-210: Great API improvement with optional parameters.Making
contextandstack_traceparameters optional provides better flexibility for callers and follows Python best practices.
219-219: Consistent handling of optional stack_trace parameter.Using
stack_trace or ""ensures consistent string handling while maintaining backward compatibility.
351-351: Appropriate default for stack_trace in error monitoring.Setting an empty string is reasonable here since the comment suggests traceback could be added later if needed.
pocketoptionapi_async/client.py (6)
150-150: Excellent API flexibility improvement.Making the
persistentparameter optional allows callers to override the instance setting when needed, providing better control over connection behavior.
165-165: Good defensive programming with explicit type conversion.The explicit
bool()conversion ensures type safety and prevents potential issues ifpersistentis passed as a different truthy/falsy type.
1189-1193: Improved candle data parsing with proper metadata extraction.The enhanced parsing correctly extracts asset and timeframe from the request ID and passes them to the parsing method, ensuring candle objects have proper metadata.
1233-1233: Better method signature with explicit parameters.Passing
assetandperiodexplicitly to_parse_stream_candlesimproves code clarity and ensures the parsing method has access to the required metadata.
1245-1247: Enhanced method signature for better data handling.The updated
_parse_stream_candlesmethod signature now explicitly acceptsassetandtimeframeparameters, improving code organization and enabling proper metadata assignment to candle objects.
1262-1263: Excellent metadata assignment to candle objects.Adding
assetandtimeframeattributes toCandleinstances ensures each candle has proper context information, which is valuable for downstream analysis and processing.Also applies to: 1274-1275
| from typing import Optional | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move the import to module level.
The Optional import should be placed at the top of the file with other imports, not inside the class definition. This is against Python conventions and could cause issues.
-from typing import Dict, List
+from typing import Dict, List, OptionalAnd remove the import from inside the class:
- from typing import Optional
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from typing import Optional | |
| # At the top of pocketoptionapi_async/constants.py | |
| from typing import Dict, List, Optional | |
| # … other imports … | |
| class Constants: | |
| # (Removed the inline `from typing import Optional`) | |
| def get_region(self) -> Optional[str]: | |
| # existing implementation… | |
| ... |
🤖 Prompt for AI Agents
In pocketoptionapi_async/constants.py around lines 181 to 182, the import of
Optional is inside the class definition which violates Python conventions. Move
the import statement for Optional to the top of the file with the other imports
and remove the import from inside the class to follow proper module-level import
practices.
| from typing import Optional | ||
|
|
||
| def __init__(self, message: str, error_code: Optional[str] = None): |
There was a problem hiding this comment.
Move import to module level.
The Optional import should be at the module level, not inside the class definition. This violates Python conventions and could cause import issues.
Apply this diff to fix the import placement:
+from typing import Optional
+
+
class PocketOptionError(Exception):
"""Base exception for all PocketOption API errors"""
- from typing import Optional
-
- def __init__(self, message: str, error_code: Optional[str] = None):
+ def __init__(self, message: str, error_code: Optional[str] = None):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from typing import Optional | |
| def __init__(self, message: str, error_code: Optional[str] = None): | |
| from typing import Optional | |
| class PocketOptionError(Exception): | |
| """Base exception for all PocketOption API errors""" | |
| def __init__(self, message: str, error_code: Optional[str] = None): | |
| # existing initialization logic... | |
| super().__init__(message) | |
| self.error_code = error_code |
🤖 Prompt for AI Agents
In pocketoptionapi_async/exceptions.py around lines 9 to 11, the import of
Optional is currently inside the class or function scope. Move the import
statement for Optional to the top of the module, outside and above any class or
function definitions, to follow Python conventions and avoid import issues.
|
Shoot, I forgot some files, ill resubmit in a few with updated files |
|
Great, no problem |
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Documentation