IB Fix for hanging issue #643
Conversation
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary by Korbit AI
Code Health Comments
- Log full tracebacks using
logging.exceptionto aid debugging across error handling methods.
Suppressed issues based on your team's Korbit activity
| This issue | Is similar to | Because |
|---|---|---|
|
The while loops in get_from_endpoint, post_to_endpoint, and delete_to_endpoint functions lack a maximum retry limit, potentially causing infinite loops. |
Ignored |
When you react to issues (for example, an upvote or downvote) or you fix them, Korbit will tune future reviews based on these signals.
Files scanned
| File Path | Reviewed |
|---|---|
| lumibot/data_sources/interactive_brokers_rest_data.py | ✅ |
| lumibot/brokers/interactive_brokers_rest.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ✅ Issue Categories
Category Enabled Naming ✅ Database Operations ✅ Documentation ✅ Logging ✅ Error Handling ✅ Systems and Environment ✅ Objects and Data Structures ✅ Readability and Maintainability ✅ Asynchronous Processing ✅ Design Patterns ✅ Third-Party Libraries ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
|
|
||
| else: | ||
| allow_fail = True | ||
|
|
||
| retries+=1 | ||
|
|
||
| except requests.exceptions.RequestException as e: |
There was a problem hiding this comment.
Use logging.exception for full stack trace.
Tell me more
In the get_from_endpoint, post_to_endpoint, and delete_to_endpoint methods, you're catching requests.exceptions.RequestException, which is good. However, you're not logging the full exception traceback. Consider using logging.exception(message) instead of logging.error(colored(message, "red")) to log the full stack trace. This will provide more detailed information for debugging purposes.
Backported from the strategy overhaul PR as it is a pretty important fix
Description by Korbit AI
What change is being made?
Refactor imports in
interactive_brokers_rest.py, modify method names for clarity, enhance logging functionality, and improve error handling and retry logic ininteractive_brokers_rest_data.py.Why are these changes being made?
These changes resolve a hanging issue by refining import paths for consistency and encapsulating helper methods to prevent unwanted external invocation. Logging enhancements provide better traceability during retries, and improved error handling ensures reliability by retrying problematic requests under specific error conditions.