Feat/network refactoring from master to dev #320
Conversation
…olve an issue detected by dependabot
…issues Fix/dependencies security issues
…https://github.com/InjectiveLabs/sdk-python into feat/network_refactoring_from_master_to_dev
WalkthroughThe recent updates focus on enhancing security and flexibility by refactoring the Changes
Poem
Recent Review DetailsConfiguration used: .coderabbit.yaml Files ignored due to path filters (1)
Files selected for processing (6)
Files not reviewed due to errors (2)
Files skipped from review due to trivial changes (1)
Additional Context UsedLanguageTool (141)
Additional comments not posted (14)
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 3
Actionable comments outside the diff hunks (1)
CHANGELOG.md (1)
Line range hint
58-58: Address formatting issues with multiple consecutive blank lines.- - +There are multiple consecutive blank lines which can be reduced to a single blank line to improve the readability and maintain the consistency of the document format.
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
| # the `credentials` parameter is ignored and will be deprecated soon. The value is taken directly from `network` | ||
| if credentials is not None: | ||
| warn( | ||
| "credentials parameter in AsyncClient is no longer used and will be deprecated", | ||
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
|
|
||
| self.addr = "" | ||
| self.number = 0 |
There was a problem hiding this comment.
Tip
Codebase Verification
The output from the executed script reveals extensive usage of the credentials parameter across various files in the codebase. This includes direct references in the AsyncClient class within test_async_client_deprecation_warnings.py, as well as numerous instances across various protocol buffer (gRPC) service files. The references in the protocol buffer files are mostly related to channel_credentials and call_credentials, which are standard parameters for gRPC channel and call configurations and do not directly relate to the credentials parameter being deprecated in the AsyncClient class.
However, the direct usage of the credentials parameter in the test_async_client_deprecation_warnings.py suggests that there might be a need to update the test cases to align with the new method of handling credentials through the network object, as indicated in the original review comment.
Given this information, it is clear that while the deprecation of the credentials parameter in the AsyncClient class is handled, there are still dependencies in the test suite that require attention to fully migrate away from using this deprecated parameter.
Conclusion:
- The deprecation of the
credentialsparameter in theAsyncClientclass is correctly implemented with appropriate warnings. - The test suite (
test_async_client_deprecation_warnings.py) still uses the deprecatedcredentialsparameter, which needs to be updated to use the new method through thenetworkobject.
Action Items:
- Update the test suite to remove direct usage of the deprecated
credentialsparameter and ensure it aligns with the new implementation.
Analysis chain
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [98-113]
Deprecation of credentials parameter handled correctly with warnings.
Ensure that all dependent code has been updated to use the new method of handling credentials through the network object.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining direct uses of the `credentials` parameter in the codebase.
rg --type py 'credentials='
Length of output: 125138
| ## [1.6.0] - 9999-99-99 | ||
| ### Added | ||
| - Added support for all queries in the chain 'tendermint' module | ||
| - Added support for all queries in the IBC Transfer module | ||
|
|
||
| ### Changed | ||
| - Refactored cookies management logic to use all gRPC calls' responses to update the current cookies | ||
|
|
||
| ## [1.5.0] - 2024-04-19 | ||
| ### Changed | ||
| - Refactoring in Network class to support mixed secure and insecure endpoints. | ||
| - Marked the Network parameter `use_secure_connection` as deprecated. | ||
|
|
||
| ## [1.4.2] - 2024-03-19 | ||
| ### Changed | ||
| - Updated `aiohttp` dependency version to ">=3.9.2" to solve a security vulnerability detected by Dependabot | ||
|
|
There was a problem hiding this comment.
Ensure consistency in list item markers.
- * Synchronized markets and tokens config files to add KIRA/INJ, NINJA/INJ, KATANA/INJ, BRETT/INJ and nINJ/INJ spot markets
+ - Synchronized markets and tokens config files to add KIRA/INJ, NINJA/INJ, KATANA/INJ, BRETT/INJ and nINJ/INJ spot marketsThe use of asterisks (*) for list items is inconsistent with the use of dashes (-) in other parts of the document. Consider standardizing to dashes for all list items to maintain consistency.
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.
| ## [1.6.0] - 9999-99-99 | |
| ### Added | |
| - Added support for all queries in the chain 'tendermint' module | |
| - Added support for all queries in the IBC Transfer module | |
| ### Changed | |
| - Refactored cookies management logic to use all gRPC calls' responses to update the current cookies | |
| ## [1.5.0] - 2024-04-19 | |
| ### Changed | |
| - Refactoring in Network class to support mixed secure and insecure endpoints. | |
| - Marked the Network parameter `use_secure_connection` as deprecated. | |
| ## [1.4.2] - 2024-03-19 | |
| ### Changed | |
| - Updated `aiohttp` dependency version to ">=3.9.2" to solve a security vulnerability detected by Dependabot | |
| ## [1.6.0] - 9999-99-99 | |
| ### Added | |
| - Added support for all queries in the chain 'tendermint' module | |
| - Added support for all queries in the IBC Transfer module | |
| ### Changed | |
| - Refactored cookies management logic to use all gRPC calls' responses to update the current cookies | |
| - Synchronized markets and tokens config files to add KIRA/INJ, NINJA/INJ, KATANA/INJ, BRETT/INJ and nINJ/INJ spot markets | |
| ## [1.5.0] - 2024-04-19 | |
| ### Changed | |
| - Refactoring in Network class to support mixed secure and insecure endpoints. | |
| - Marked the Network parameter `use_secure_connection` as deprecated. | |
| ## [1.4.2] - 2024-03-19 | |
| ### Changed | |
| - Updated `aiohttp` dependency version to ">=3.9.2" to solve a security vulnerability detected by Dependabot | |
| @@ -2,14 +2,23 @@ | |||
|
|
|||
| All notable changes to this project will be documented in this file. | |||
|
|
|||
| ## [1.5.0] - 9999-99-99 | |||
| ## [1.6.0] - 9999-99-99 | |||
There was a problem hiding this comment.
Add surrounding blank lines for headings.
+
## [1.6.0] - 9999-99-99
+Headings should be surrounded by blank lines to enhance the readability and structure of the document.
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.
| ## [1.6.0] - 9999-99-99 | |
| ## [1.6.0] - 9999-99-99 | |
devbranch (feat/refactor_network_mixed_secure_insecure_channels #319)Summary by CodeRabbit
grpcchannels in theNetworkclass.use_secure_connectionandcredentialsparameters in theNetworkandAsyncClientclasses respectively.aiohttpdependency to address a security vulnerability.NetworkandAsyncClientclasses.