-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix/sync_dev_after_v1_7_1 #352
Conversation
Fix/solve OFAC path issue
WalkthroughThe pull request introduces updates to the project, including a version change in Changes
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some test runs failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
pyproject.toml (1)
Line range hint
1-120
: Consider reviewing dependency versions and test coverage requirements.While not directly related to the version change, it might be beneficial to:
- Review the
aiohttp
andprotobuf
version constraints to ensure they're still appropriate.- Consider if the 65% test coverage requirement is sufficient or if it should be increased.
These suggestions are for future improvements and don't need to be addressed in this PR.
CHANGELOG.md (2)
9-11
: LGTM: Changelog entry for version 1.7.0 is informative. Minor formatting suggestion.The entry for version 1.7.0 is well-formatted and provides clear information about the added feature. For consistency with the 1.7.1 entry, consider adding a newline before the "Added" section.
Consider applying this minor formatting change:
## [1.7.0] - 2024-09-19 + ### Added - Added OFAC restricted addresses validations
Tools
LanguageTool
[duplication] ~10-~10: Possible typo: you repeated a word
Context: ... file path ## [1.7.0] - 2024-09-19 ### Added - Added OFAC restricted addresses validations ...(ENGLISH_WORD_REPEAT_RULE)
Line range hint
1-241
: LGTM: Changelog structure is consistent. Minor duplication in older entry.The overall structure and formatting of the changelog are consistent and follow best practices. The new entries for versions 1.7.1 and 1.7.0 are correctly placed at the top of the file.
There's a minor duplication in the entry for version 1.4.3. Consider removing the duplicate line:
## [1.4.3] - 2024-06-06 ### Changed - Fixed `protobuf` dependency version to "<5" to for the v1.4 branch, because newer versions require a code refactoring (done in v1.5) -- Fixed `protobuf` dependency version to "<5" for the v1.4 branch because newer versions require a code refactoring (done in v1.5)
Tools
LanguageTool
[duplication] ~6-~6: Possible typo: you repeated a word
Context: ...this file. ## [1.7.1] - 2024-09-24 ### Fixed - Fixed logic to get the absolute ofac.json fil...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~10-~10: Possible typo: you repeated a word
Context: ... file path ## [1.7.0] - 2024-09-19 ### Added - Added OFAC restricted addresses validations ...(ENGLISH_WORD_REPEAT_RULE)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- pyinjective/ofac.py (1 hunks)
- pyproject.toml (1 hunks)
Additional context used
LanguageTool
CHANGELOG.md
[duplication] ~6-~6: Possible typo: you repeated a word
Context: ...this file. ## [1.7.1] - 2024-09-24 ### Fixed - Fixed logic to get the absolute ofac.json fil...(ENGLISH_WORD_REPEAT_RULE)
Ruff
pyinjective/ofac.py
19-21: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
Additional comments not posted (4)
pyinjective/ofac.py (2)
25-25
: Simplified and improved path resolution.The
get_ofac_list_path
method has been streamlined to useos.path.dirname(__file__)
, which provides a more reliable way to locate the OFAC list file. This change simplifies the code and reduces the potential for errors in path resolution.
Line range hint
1-53
: Overall improvements in error handling and file path resolution.The changes in this file align well with the PR objective of synchronizing the dev branch with master. The improvements in error handling and the simplification of the file path resolution contribute to the overall robustness and maintainability of the codebase.
Tools
Ruff
19-21: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
pyproject.toml (1)
3-3
: Version update looks good and aligns with PR objectives.The version change from "1.7.0" to "1.8.0-pre" is appropriate for synchronizing the dev branch after the v1.7.1 release. This pre-release version sets the stage for future development work towards the next major release (1.8.0).
CHANGELOG.md (1)
5-7
: LGTM: New changelog entry for version 1.7.1 is well-formatted and informative.The new entry for version 1.7.1 is correctly formatted and provides clear information about the fix implemented in this release. The date is consistent with the PR objectives.
Tools
LanguageTool
[duplication] ~6-~6: Possible typo: you repeated a word
Context: ...this file. ## [1.7.1] - 2024-09-24 ### Fixed - Fixed logic to get the absolute ofac.json fil...(ENGLISH_WORD_REPEAT_RULE)
try: | ||
with open(self._ofac_list_path, "r") as f: | ||
self._ofac_list = set(json.load(f)) | ||
except Exception as e: | ||
raise Exception( | ||
"OFAC list is missing on the disk. Please, download it by running python3 pyinjective/ofac_list.py" | ||
f"Error parsing OFAC list. Please, download it by running python3 pyinjective/ofac_list.py ({e})" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved error handling, but consider preserving the original exception.
The new try-except block enhances error handling when reading and parsing the OFAC list. The error message is now more informative, guiding users to download the list if needed.
To further improve exception handling and preserve the original traceback, consider using raise ... from e
:
try:
with open(self._ofac_list_path, "r") as f:
self._ofac_list = set(json.load(f))
except Exception as e:
- raise Exception(
- f"Error parsing OFAC list. Please, download it by running python3 pyinjective/ofac_list.py ({e})"
- )
+ raise Exception(
+ f"Error parsing OFAC list. Please, download it by running python3 pyinjective/ofac_list.py"
+ ) from e
This change will help maintain the full context of the original error while providing the custom error message.
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.
try: | |
with open(self._ofac_list_path, "r") as f: | |
self._ofac_list = set(json.load(f)) | |
except Exception as e: | |
raise Exception( | |
"OFAC list is missing on the disk. Please, download it by running python3 pyinjective/ofac_list.py" | |
f"Error parsing OFAC list. Please, download it by running python3 pyinjective/ofac_list.py ({e})" | |
try: | |
with open(self._ofac_list_path, "r") as f: | |
self._ofac_list = set(json.load(f)) | |
except Exception as e: | |
raise Exception( | |
f"Error parsing OFAC list. Please, download it by running python3 pyinjective/ofac_list.py" | |
) from e |
master
after releasing version 1.7.1 into devSummary by CodeRabbit
ofac.json
file in version 1.7.1.pyproject.toml
.