Conversation
update to fix blind exceptions
|
Warning Rate limit exceeded@chrisaddy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 36 seconds before requesting another review. ⌛ 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 (12)
WalkthroughThis update introduces explicit type annotations across multiple modules, refines exception handling in FastAPI endpoints, and enforces HTTP request timeouts. Project dependencies and linting configurations are expanded, and command-line tasks are updated to use the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FastAPI_App
participant ExternalService
Client->>FastAPI_App: Send API request (e.g., /equity-bars)
FastAPI_App->>ExternalService: Make HTTP request with 30s timeout
ExternalService-->>FastAPI_App: Respond (or error)
alt Success
FastAPI_App-->>Client: Return result
else Specific Error (Requests, API, Validation, etc.)
FastAPI_App-->>Client: Return HTTP 500 with error message
end
Suggested labels
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
🛑 Comments failed to post (5)
application/datamanager/src/datamanager/models.py (1)
32-34:
⚠️ Potential issueMismatch in type annotation: use
datetime.dateinstead ofdatetime.datetime.
TheDateRangemodel fields are declared asdatetime.date, so the validator will receive and should return adate, not adatetime. Update the signature accordingly:@field_validator("end") @classmethod -def check_end_after_start( - cls, end_value: datetime.datetime, info: core_schema.ValidationInfo -) -> datetime.datetime: +def check_end_after_start( + cls, end_value: datetime.date, info: core_schema.ValidationInfo +) -> datetime.date:📝 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.@field_validator("end") @classmethod def check_end_after_start( cls, end_value: datetime.date, info: core_schema.ValidationInfo ) -> datetime.date: start_value = info.data.get("start")🤖 Prompt for AI Agents
In application/datamanager/src/datamanager/models.py around lines 32 to 34, the method signature uses datetime.datetime for the end_value parameter and return type, but the DateRange model fields are datetime.date. Change the type annotations from datetime.datetime to datetime.date for both the parameter and the return type to match the model's field types.application/datamanager/src/datamanager/config.py (1)
8-8:
⚠️ Potential issueType annotations are more accurate, but potential runtime issue detected.
The change from
strtostr | Nonecorrectly reflects thatos.getenv()can returnNonewhen environment variables are not set. However, this introduces a potential runtime issue with thedaily_bars_pathcomputed field on line 19.Issue: The
daily_bars_pathmethod usesself.namein an f-string, which will raise aTypeErrorifnameisNone:return f"gs://{self.name}/equity/bars/" # Fails if self.name is NoneSuggested fix: Handle the
Nonecase in the computed field:@computed_field def daily_bars_path(self) -> str: + if self.name is None: + raise ValueError("Bucket name is required but not configured") return f"gs://{self.name}/equity/bars/"Alternatively, you could make this field optional and return
Nonewhennameis not set.Also applies to: 14-15
🤖 Prompt for AI Agents
In application/datamanager/src/datamanager/config.py around lines 8 and 14-19, the type annotation for api_key was changed to str | None to reflect that os.getenv() can return None, but this causes a runtime error in the daily_bars_path computed field when self.name is None. To fix this, modify the daily_bars_path method to check if self.name is None before using it in the f-string, and either return None or a safe default value when name is not set, preventing a TypeError.application/positionmanager/src/positionmanager/clients.py (1)
15-16: 🛠️ Refactor suggestion
Type annotation inconsistency with default values.
The parameters now accept
str | Nonebut default to empty strings, which creates confusion. IfNoneis an acceptable value, consider defaulting toNoneinstead of empty strings for clarity, or removeNonefrom the type annotation if empty strings are the intended default.Consider this approach for better consistency:
- api_key: str | None = "", - api_secret: str | None = "", + api_key: str | None = None, + api_secret: str | None = None,This aligns the type annotation with the actual intention and makes the validation logic more explicit.
📝 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.api_key: str | None = None, api_secret: str | None = None,🤖 Prompt for AI Agents
In application/positionmanager/src/positionmanager/clients.py around lines 15 to 16, the parameters api_key and api_secret are annotated as str | None but default to empty strings, causing inconsistency. To fix this, either change the default values to None to match the type annotation or remove None from the type annotation if empty strings are the intended defaults. This ensures the type hints align with the actual default values and clarifies the expected input.application/datamanager/src/datamanager/main.py (2)
114-120:
⚠️ Potential issueFix typo in exception class name.
There's a typo in the exception handling -
requests.RequestsExceptionshould berequests.RequestException.Apply this diff to fix the typo:
- requests.RequestsException, + requests.RequestException,📝 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.except ( requests.RequestException, ComputeError, IOException, GoogleAPIError, pyarrow.lib.ArrowIOError, ) as e:🤖 Prompt for AI Agents
In application/datamanager/src/datamanager/main.py around lines 114 to 120, the exception class name is incorrectly written as requests.RequestsException. Change it to the correct requests.RequestException to properly catch the intended exceptions.
157-163:
⚠️ Potential issueFix typo in exception class name (duplicate issue).
Same typo as in the GET endpoint -
requests.RequestsExceptionshould berequests.RequestException.Apply this diff to fix the typo:
- requests.RequestsException, + requests.RequestException,📝 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.except ( requests.RequestException, ComputeError, IOException, GoogleAPIError, pyarrow.lib.ArrowIOError, ) as e:🤖 Prompt for AI Agents
In application/datamanager/src/datamanager/main.py around lines 157 to 163, the exception class name is incorrectly written as requests.RequestsException. Change it to the correct requests.RequestException to fix the typo and properly catch the intended exception.
add bugbear add comma linting fix bugbear and commas add timezones
fix boolean traps
Commas & time zones
2 participants
This pull request introduces several updates across multiple files to improve type safety, enhance error handling, and refine functionality in the
datamanagerandpositionmanagerservices. Key changes include the addition of type annotations, improved exception handling, and updates to configuration and validation logic.Type Safety Enhancements:
application/datamanager/features/environment.pyandapplication/datamanager/features/steps/equity_bars_steps.pyto improve code clarity and enforce type checks. [1] [2]application/datamanager/src/datamanager/models.pyandapplication/positionmanager/src/positionmanager/models.pyto use type annotations for input parameters and validation logic. [1] [2] [3] [4]Error Handling Improvements:
application/datamanager/src/datamanager/main.pyandapplication/positionmanager/src/positionmanager/main.pyby replacing genericExceptionblocks with specific exceptions likerequests.RequestException,ComputeError, andValidationError. [1] [2] [3] [4] [5]Configuration and Validation Updates:
application/datamanager/src/datamanager/config.pyandapplication/positionmanager/src/positionmanager/clients.pyto use optional types (str | None) for better handling of missing values. [1] [2] [3]application/datamanager/src/datamanager/models.pyandapplication/positionmanager/src/positionmanager/models.pyto ensure logical consistency (e.g., end date must be after start date). [1] [2]Dependency and Timeout Adjustments:
uvxto enhance task execution in.mise.toml, and added a timeout parameter to HTTP requests inapplication/datamanager/features/steps/equity_bars_steps.pyandapplication/datamanager/features/steps/health_steps.pyfor better reliability. [1] [2] [3] [4]Miscellaneous Changes:
application/datamanager/src/datamanager/main.pyandapplication/positionmanager/src/positionmanager/main.pyto improve organization and readability. [1] [2] [3]application/positionmanager/tests/test_positionmanager_main.pyto include type annotations for better test maintainability.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores