Conversation
added 2 commits
March 19, 2024 10:14
…assistant and replaced it with a general ExpiringCookieAssistant. Updated all unit tests.
Contributor
WalkthroughThe update involves a significant overhaul of the codebase, focusing on centralizing cookies management and metadata handling for gRPC calls. This restructuring emphasizes direct method calls over lambda functions and introduces a structured approach to managing cookies and metadata through the Changes
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #314 +/- ##
==========================================
+ Coverage 76.40% 76.98% +0.58%
==========================================
Files 51 51
Lines 4170 4145 -25
Branches 353 309 -44
==========================================
+ Hits 3186 3191 +5
+ Misses 878 874 -4
+ Partials 106 80 -26 ☔ View full report in Codecov by Sentry. |
Contributor
There was a problem hiding this comment.
Review Status
Actionable comments generated: 7
Configuration used: .coderabbit.yaml
Files selected for processing (62)
- CHANGELOG.md (1 hunks)
- pyinjective/async_client.py (23 hunks)
- pyinjective/client/chain/grpc/chain_grpc_auction_api.py (2 hunks)
- pyinjective/client/chain/grpc/chain_grpc_auth_api.py (1 hunks)
- pyinjective/client/chain/grpc/chain_grpc_authz_api.py (1 hunks)
- pyinjective/client/chain/grpc/chain_grpc_bank_api.py (1 hunks)
- pyinjective/client/chain/grpc/chain_grpc_distribution_api.py (2 hunks)
- pyinjective/client/chain/grpc/chain_grpc_exchange_api.py (2 hunks)
- pyinjective/client/chain/grpc/chain_grpc_token_factory_api.py (2 hunks)
- pyinjective/client/chain/grpc/chain_grpc_wasm_api.py (1 hunks)
- pyinjective/client/chain/grpc_stream/chain_grpc_chain_stream.py (1 hunks)
- pyinjective/client/indexer/grpc/indexer_grpc_account_api.py (2 hunks)
- pyinjective/client/indexer/grpc/indexer_grpc_auction_api.py (2 hunks)
- pyinjective/client/indexer/grpc/indexer_grpc_derivative_api.py (2 hunks)
- pyinjective/client/indexer/grpc/indexer_grpc_explorer_api.py (2 hunks)
- pyinjective/client/indexer/grpc/indexer_grpc_insurance_api.py (2 hunks)
- pyinjective/client/indexer/grpc/indexer_grpc_meta_api.py (2 hunks)
- pyinjective/client/indexer/grpc/indexer_grpc_oracle_api.py (2 hunks)
- pyinjective/client/indexer/grpc/indexer_grpc_portfolio_api.py (2 hunks)
- pyinjective/client/indexer/grpc/indexer_grpc_spot_api.py (2 hunks)
- pyinjective/client/indexer/grpc_stream/indexer_grpc_account_stream.py (2 hunks)
- pyinjective/client/indexer/grpc_stream/indexer_grpc_auction_stream.py (2 hunks)
- pyinjective/client/indexer/grpc_stream/indexer_grpc_derivative_stream.py (2 hunks)
- pyinjective/client/indexer/grpc_stream/indexer_grpc_explorer_stream.py (2 hunks)
- pyinjective/client/indexer/grpc_stream/indexer_grpc_meta_stream.py (2 hunks)
- pyinjective/client/indexer/grpc_stream/indexer_grpc_oracle_stream.py (2 hunks)
- pyinjective/client/indexer/grpc_stream/indexer_grpc_portfolio_stream.py (2 hunks)
- pyinjective/client/indexer/grpc_stream/indexer_grpc_spot_stream.py (2 hunks)
- pyinjective/core/network.py (12 hunks)
- pyinjective/core/tx/grpc/tendermint_grpc_api.py (2 hunks)
- pyinjective/core/tx/grpc/tx_grpc_api.py (1 hunks)
- pyinjective/utils/grpc_api_request_assistant.py (1 hunks)
- pyinjective/utils/grpc_api_stream_assistant.py (2 hunks)
- tests/client/chain/grpc/test_chain_grpc_auction_api.py (6 hunks)
- tests/client/chain/grpc/test_chain_grpc_auth_api.py (5 hunks)
- tests/client/chain/grpc/test_chain_grpc_authz_api.py (5 hunks)
- tests/client/chain/grpc/test_chain_grpc_bank_api.py (14 hunks)
- tests/client/chain/grpc/test_chain_grpc_distribution_api.py (11 hunks)
- tests/client/chain/grpc/test_chain_grpc_exchange_api.py (58 hunks)
- tests/client/chain/grpc/test_chain_grpc_token_factory_api.py (6 hunks)
- tests/client/chain/grpc/test_chain_grpc_wasm_api.py (13 hunks)
- tests/client/chain/stream_grpc/test_chain_grpc_chain_stream.py (3 hunks)
- tests/client/indexer/grpc/test_indexer_grpc_account_api.py (11 hunks)
- tests/client/indexer/grpc/test_indexer_grpc_auction_api.py (4 hunks)
- tests/client/indexer/grpc/test_indexer_grpc_derivative_api.py (19 hunks)
- tests/client/indexer/grpc/test_indexer_grpc_explorer_api.py (21 hunks)
- tests/client/indexer/grpc/test_indexer_grpc_insurance_api.py (4 hunks)
- tests/client/indexer/grpc/test_indexer_grpc_meta_api.py (5 hunks)
- tests/client/indexer/grpc/test_indexer_grpc_oracle_api.py (4 hunks)
- tests/client/indexer/grpc/test_indexer_grpc_portfolio_api.py (4 hunks)
- tests/client/indexer/grpc/test_indexer_grpc_spot_api.py (13 hunks)
- tests/client/indexer/stream_grpc/test_indexer_grpc_account_stream.py (3 hunks)
- tests/client/indexer/stream_grpc/test_indexer_grpc_auction_stream.py (3 hunks)
- tests/client/indexer/stream_grpc/test_indexer_grpc_derivative_stream.py (10 hunks)
- tests/client/indexer/stream_grpc/test_indexer_grpc_explorer_stream.py (4 hunks)
- tests/client/indexer/stream_grpc/test_indexer_grpc_meta_stream.py (3 hunks)
- tests/client/indexer/stream_grpc/test_indexer_grpc_oracle_stream.py (4 hunks)
- tests/client/indexer/stream_grpc/test_indexer_grpc_portfolio_stream.py (3 hunks)
- tests/client/indexer/stream_grpc/test_indexer_grpc_spot_stream.py (9 hunks)
- tests/core/test_network.py (1 hunks)
- tests/core/tx/grpc/test_tendermint_grpc_api.py (9 hunks)
- tests/core/tx/grpc/test_tx_grpc_api.py (5 hunks)
Additional comments: 105
pyinjective/utils/grpc_api_request_assistant.py (2)
- 9-11: The constructor now correctly initializes
CookieAssistant, enhancing cookie management by directly integrating it into the request execution process.- 14-18: The
execute_callmethod effectively utilizesCookieAssistantfor managing metadata and processing response metadata. This aligns with the PR's objective of optimizing cookie processing.pyinjective/client/indexer/grpc_stream/indexer_grpc_meta_stream.py (1)
- 14-16: Replacing
metadata_providerwithcookie_assistantin the constructor and usingCookieAssistantfor cookie management is a positive change. It aligns with the PR's goal of introducing a more generic and efficient cookie handling mechanism.pyinjective/client/indexer/grpc_stream/indexer_grpc_auction_stream.py (1)
- 14-16: The introduction of
CookieAssistantin the constructor for cookie management is consistent with the PR's objectives. This change should enhance the flexibility and efficiency of cookie handling in the auction stream context.pyinjective/client/indexer/grpc/indexer_grpc_auction_api.py (1)
- 14-16: The update to use
CookieAssistantin the__init__method for the auction API is a significant improvement. It ensures that cookie management is handled more efficiently, aligning with the PR's goals of optimization and simplification.pyinjective/client/indexer/grpc_stream/indexer_grpc_account_stream.py (1)
- 14-16: The modification to use
CookieAssistantinstead ofmetadata_providerin the account stream constructor is in line with the PR's objectives. This change should improve cookie management efficiency and flexibility.pyinjective/client/indexer/grpc_stream/indexer_grpc_portfolio_stream.py (1)
- 14-16: Integrating
CookieAssistantinto the portfolio stream's constructor is a commendable change. It ensures more efficient and flexible cookie management, supporting the PR's aim of enhancing cookie handling mechanisms.pyinjective/client/indexer/grpc/indexer_grpc_portfolio_api.py (1)
- 14-16: The update to incorporate
CookieAssistantin the portfolio API's constructor is a positive step towards achieving the PR's objectives of optimizing and simplifying cookie management.pyinjective/client/indexer/grpc/indexer_grpc_meta_api.py (1)
- 15-17: Replacing
metadata_providerwithcookie_assistantin the meta API's constructor aligns with the PR's goals of enhancing cookie management. This change should contribute to the system's overall efficiency and flexibility in handling cookies.pyinjective/core/tx/grpc/tx_grpc_api.py (2)
- 11-11: The constructor now correctly accepts a
CookieAssistantinstance instead of ametadata_provider. This aligns with the PR's objective to centralize and optimize cookie management.- 13-13: Initialization of
GrpcApiRequestAssistantwithcookie_assistantis consistent with the new approach to cookie management. This change should facilitate more efficient cookie handling across gRPC calls.pyinjective/client/chain/grpc/chain_grpc_auction_api.py (2)
- 14-14: Replacing
metadata_providerwithcookie_assistantin the constructor is a positive change, ensuring consistency in cookie management across the system.- 16-16: The update to use
cookie_assistantinGrpcApiRequestAssistantinitialization is appropriate and aligns with the PR's goals for efficient cookie handling.pyinjective/client/indexer/grpc/indexer_grpc_insurance_api.py (2)
- 14-14: The change to use
cookie_assistantin the constructor instead ofmetadata_provideris consistent with the PR's objectives and enhances cookie management flexibility.- 16-16: Initialization of
GrpcApiRequestAssistantwithcookie_assistantis correctly implemented, supporting the PR's goal of optimizing cookie retrieval and management.pyinjective/utils/grpc_api_stream_assistant.py (2)
- 11-11: The constructor's update to accept a
CookieAssistantobject enhances the system's cookie management capabilities, aligning with the PR's objectives.- 23-23: Using
self._cookie_assistant.metadata()in thelisten_streammethod is a significant improvement for retrieving metadata, ensuring efficient cookie handling in streaming scenarios.pyinjective/client/chain/grpc/chain_grpc_auth_api.py (2)
- 12-12: Switching to
cookie_assistantin the constructor is a strategic move that supports the PR's goal of centralizing and optimizing cookie management.- 14-14: The update to initialize
GrpcApiRequestAssistantwithcookie_assistantis correctly implemented, facilitating efficient cookie handling.pyinjective/client/indexer/grpc/indexer_grpc_oracle_api.py (2)
- 14-14: Adopting
cookie_assistantin the constructor instead ofmetadata_provideraligns with the PR's objectives for improved cookie management.- 16-16: Correctly initializing
GrpcApiRequestAssistantwithcookie_assistantsupports the PR's goal of optimizing cookie retrieval and management.pyinjective/client/indexer/grpc_stream/indexer_grpc_explorer_stream.py (1)
- 14-14: Replacing
metadata_providerwithcookie_assistantin the constructor is a positive step towards centralizing cookie management, in line with the PR's objectives.pyinjective/client/indexer/grpc_stream/indexer_grpc_oracle_stream.py (2)
- 14-14: The change to use
cookie_assistantin the constructor enhances the system's cookie management capabilities, aligning with the PR's objectives.- 16-16: Initializing
GrpcApiStreamAssistantwithcookie_assistantis correctly implemented, supporting the PR's goal of optimizing cookie retrieval and management.pyinjective/client/chain/grpc/chain_grpc_token_factory_api.py (1)
- 5-5: The import of
CookieAssistantis correctly added to support the new cookie handling mechanism.tests/client/indexer/stream_grpc/test_indexer_grpc_meta_stream.py (3)
- 7-7: The import of
DisabledCookieAssistantandNetworkis correctly added to support the new cookie handling mechanism in tests.- 35-35: The use of
_api_instancemethod to create the API instance in the test function is a good practice. It centralizes the setup logic, improving maintainability and readability.- 58-66: The
_api_instancemethod is well-implemented, providing a centralized way to configure API instances for tests. This approach enhances test maintainability and readability. Ensure that all test functions in this file and similar test files use this method for API instance creation.tests/client/indexer/stream_grpc/test_indexer_grpc_auction_stream.py (3)
- 7-7: The import of
DisabledCookieAssistantandNetworkis correctly added to support the new cookie handling mechanism in tests.- 32-32: The use of
_api_instancemethod to create the API instance in the test function is a good practice. It centralizes the setup logic, improving maintainability and readability.- 60-68: The
_api_instancemethod is well-implemented, providing a centralized way to configure API instances for tests. This approach enhances test maintainability and readability. Ensure that all test functions in this file and similar test files use this method for API instance creation.pyinjective/client/chain/grpc/chain_grpc_authz_api.py (2)
- 6-6: The import of
CookieAssistantis correctly added to support the new cookie handling mechanism.- 12-14: The
__init__method ofChainGrpcAuthZApihas been updated to usecookie_assistantinstead ofmetadata_provider. This change aligns with the PR's objective to optimize cookie handling. Ensure that all instances ofChainGrpcAuthZApithroughout the codebase are updated to pass theCookieAssistantinstance.pyinjective/client/chain/grpc_stream/chain_grpc_chain_stream.py (2)
- 5-5: The import of
CookieAssistantis correctly added to support the new cookie handling mechanism.- 11-13: The
__init__method ofChainGrpcChainStreamhas been updated to usecookie_assistantinstead ofmetadata_provider. This change aligns with the PR's objective to optimize cookie handling. Ensure that all instances ofChainGrpcChainStreamthroughout the codebase are updated to pass theCookieAssistantinstance.tests/client/indexer/grpc/test_indexer_grpc_oracle_api.py (3)
- 5-5: The import of
DisabledCookieAssistantandNetworkis correctly added to support the new cookie handling mechanism in tests.- 35-35: The use of
_api_instancemethod to create the API instance in the test functions is a good practice. It centralizes the setup logic, improving maintainability and readability.Also applies to: 65-65
- 77-85: The
_api_instancemethod is well-implemented, providing a centralized way to configure API instances for tests. This approach enhances test maintainability and readability. Ensure that all test functions in this file and similar test files use this method for API instance creation.tests/client/indexer/stream_grpc/test_indexer_grpc_portfolio_stream.py (3)
- 7-7: The import of
DisabledCookieAssistantandNetworkis correctly added to support the new cookie handling mechanism in tests.- 39-39: The use of
_api_instancemethod to create the API instance in the test function is a good practice. It centralizes the setup logic, improving maintainability and readability.- 71-79: The
_api_instancemethod is well-implemented, providing a centralized way to configure API instances for tests. This approach enhances test maintainability and readability. Ensure that all test functions in this file and similar test files use this method for API instance creation.tests/client/indexer/grpc/test_indexer_grpc_meta_api.py (3)
- 5-5: The import of
DisabledCookieAssistantandNetworkis correctly added to support the new cookie handling mechanism in tests.- 23-23: The use of
_api_instancemethod to create the API instance in the test functions is a good practice. It centralizes the setup logic, improving maintainability and readability.Also applies to: 47-47, 73-73
- 86-94: The
_api_instancemethod is well-implemented, providing a centralized way to configure API instances for tests. This approach enhances test maintainability and readability. Ensure that all test functions in this file and similar test files use this method for API instance creation.pyinjective/core/tx/grpc/tendermint_grpc_api.py (1)
- 15-17: The replacement of
metadata_providerwithcookie_assistantin the constructor and the subsequent use ofcookie_assistantin initializingGrpcApiRequestAssistantaligns with the PR's objective to optimize cookie handling. This change centralizes cookie management and simplifies the codebase.tests/client/indexer/stream_grpc/test_indexer_grpc_account_stream.py (2)
- 37-37: Refactoring the
test_fetch_portfoliomethod to use the_api_instancemethod for creating theIndexerGrpcAccountStreaminstance simplifies the test setup and enhances code readability. This change effectively utilizes theDisabledCookieAssistant, aligning with the PR's goal of introducing a more generic cookie assistant.- 73-81: The introduction of the
_api_instancemethod centralizes the setup logic for creatingIndexerGrpcAccountStreaminstances in tests. This approach not only improves code maintainability but also ensures consistency in how instances are created across different tests.tests/core/test_network.py (3)
- 24-41: The test case for
BareMetalLoadBalancedCookieAssistantcorrectly assesses the functionality of metadata access and processing response metadata. This test ensures that the assistant can handle cookies as expected, which is crucial for the optimization of cookie processing.- 46-86: The test cases for
ExpiringCookieAssistanteffectively validate cookie expiration handling and instance creation for Kubernetes servers. These tests are essential for verifying the assistant's ability to manage cookies with expiration times, supporting the PR's goal of introducing a generic cookie assistant.- 91-94: The test case for
DisabledCookieAssistantconfirms that it correctly returns an empty metadata object. This behavior is consistent with the assistant's purpose of disabling cookie management, aligning with the PR's objective of flexibility in cookie handling.tests/client/indexer/stream_grpc/test_indexer_grpc_oracle_stream.py (3)
- 33-33: Refactoring the
test_stream_oracle_pricesmethod to use the_api_instancemethod for creatingIndexerGrpcOracleStreaminstances simplifies the test setup and enhances code readability. This change effectively utilizes theDisabledCookieAssistant, aligning with the PR's goal of introducing a more generic cookie assistant.- 76-76: The refactoring of the
test_stream_oracle_prices_by_marketsmethod to use the_api_instancemethod is a good practice, as it centralizes the setup logic and improves the maintainability of the test code.- 100-108: The introduction of the
_api_instancemethod inTestIndexerGrpcOracleStreamcentralizes the setup logic for creatingIndexerGrpcOracleStreaminstances in tests. This approach not only improves code maintainability but also ensures consistency in how instances are created across different tests.tests/client/indexer/grpc/test_indexer_grpc_auction_api.py (3)
- 47-47: Refactoring the
test_fetch_auctionmethod to use the_api_instancemethod for creatingIndexerGrpcAuctionApiinstances simplifies the test setup and enhances code readability. This change effectively utilizes theDisabledCookieAssistant, aligning with the PR's goal of introducing a more generic cookie assistant.- 89-89: The refactoring of the
test_fetch_auctionsmethod to use the_api_instancemethod is a good practice, as it centralizes the setup logic and improves the maintainability of the test code.- 112-120: The introduction of the
_api_instancemethod inTestIndexerGrpcAuctionApicentralizes the setup logic for creatingIndexerGrpcAuctionApiinstances in tests. This approach not only improves code maintainability but also ensures consistency in how instances are created across different tests.pyinjective/client/indexer/grpc/indexer_grpc_account_api.py (1)
- 15-17: The replacement of
metadata_providerwithcookie_assistantin the constructor and the subsequent use ofcookie_assistantin initializingGrpcApiRequestAssistantaligns with the PR's objective to optimize cookie handling. This change centralizes cookie management and simplifies the codebase.pyinjective/client/chain/grpc/chain_grpc_distribution_api.py (1)
- 15-17: The replacement of
metadata_providerwithcookie_assistantin the constructor and the subsequent use ofcookie_assistantin initializingGrpcApiRequestAssistantaligns with the PR's objective to optimize cookie handling. This change centralizes cookie management and simplifies the codebase.tests/client/indexer/stream_grpc/test_indexer_grpc_explorer_stream.py (3)
- 44-44: Refactoring the
test_stream_txsmethod to use the_api_instancemethod for creatingIndexerGrpcExplorerStreaminstances simplifies the test setup and enhances code readability. This change effectively utilizes theDisabledCookieAssistant, aligning with the PR's goal of introducing a more generic cookie assistant.- 96-96: The refactoring of the
test_stream_blocksmethod to use the_api_instancemethod is a good practice, as it centralizes the setup logic and improves the maintainability of the test code.- 129-137: The introduction of the
_api_instancemethod inTestIndexerGrpcAuctionStreamcentralizes the setup logic for creatingIndexerGrpcExplorerStreaminstances in tests. This approach not only improves code maintainability but also ensures consistency in how instances are created across different tests.pyinjective/client/chain/grpc/chain_grpc_bank_api.py (1)
- 12-14: The changes to use
CookieAssistantin theChainGrpcBankApiclass are aligned with the PR's objectives to optimize cookie handling. The implementation is correct and no further action is required.tests/client/indexer/grpc/test_indexer_grpc_insurance_api.py (1)
- 114-122: The introduction of the
_api_instancemethod in the test file is a good practice for centralizing the setup logic forIndexerGrpcInsuranceApiinstances. This change simplifies the test setup and enhances code readability and maintainability. No further action is required.tests/client/chain/grpc/test_chain_grpc_token_factory_api.py (1)
- 142-150: The
_api_instancemethod in this test file is well-implemented, centralizing the setup logic forChainGrpcTokenFactoryApiinstances. This approach enhances code readability and maintainability. No further action is required.pyinjective/client/chain/grpc/chain_grpc_wasm_api.py (1)
- 12-14: The changes to use
CookieAssistantin theChainGrpcWasmApiclass are aligned with the PR's objectives to optimize cookie handling. The implementation is correct and no further action is required.tests/client/chain/grpc/test_chain_grpc_auction_api.py (1)
- 149-157: The introduction of the
_api_instancemethod in the test file is a good practice for centralizing the setup logic forChainGrpcAuctionApiinstances. This change simplifies the test setup and enhances code readability and maintainability. No further action is required.tests/client/chain/grpc/test_chain_grpc_auth_api.py (1)
- 161-169: The
_api_instancemethod in this test file is well-implemented, centralizing the setup logic forChainGrpcAuthApiinstances. This approach enhances code readability and maintainability. No further action is required.tests/client/chain/grpc/test_chain_grpc_authz_api.py (1)
- 189-197: The introduction of the
_api_instancemethod in the test file is a good practice for centralizing the setup logic forChainGrpcAuthZApiinstances. This change simplifies the test setup and enhances code readability and maintainability. No further action is required.tests/client/indexer/grpc/test_indexer_grpc_portfolio_api.py (3)
- 5-5: The inclusion of
DisabledCookieAssistantaligns with the PR's objective to introduce a more generic cookie assistant. This change enhances the flexibility of cookie management across different environments.- 66-66: Refactoring test methods to use the
_api_instancemethod for creating instances ofIndexerGrpcPortfolioApiis a significant improvement. It centralizes the instantiation logic, which enhances code readability and maintainability. This approach also simplifies the setup for test methods, making it easier to manage changes in the instantiation process in the future.Also applies to: 143-143
- 170-178: The implementation of
_api_instanceis clear and concise. It correctly sets up theIndexerGrpcPortfolioApiinstance with a disabled cookie assistant and a gRPC channel based on theNetwork.devnet()configuration. However, it's important to ensure that theservicerpassed to_api_instanceis always compatible with the expected stub interface to avoid runtime errors.tests/core/tx/grpc/test_tx_grpc_api.py (2)
- 194-202: The introduction of the
_api_instancemethod centralizes the creation ofTxGrpcApiinstances, which is a good practice for maintainability and readability.- 40-40: The usage of
_api_instancein test methods simplifies the test setup by removing repetitive code, which is a positive change for code maintainability and readability.Also applies to: 99-99, 169-169
pyinjective/client/indexer/grpc_stream/indexer_grpc_spot_stream.py (1)
- 15-17: Replacing
metadata_providerwithcookie_assistantin the constructor aligns with the PR's objective to introduce a generic cookie assistant, enhancing flexibility and applicability across different environments.pyinjective/client/indexer/grpc_stream/indexer_grpc_derivative_stream.py (1)
- 15-17: Replacing
metadata_providerwithcookie_assistantin the constructor is consistent with the PR's objectives, improving the code's flexibility and maintainability.pyinjective/client/indexer/grpc/indexer_grpc_spot_api.py (2)
- 15-17: The change to use
CookieAssistantinstead ofCallablefor thecookie_assistantparameter is in line with the PR's objectives, enhancing the code's flexibility and maintainability.- 6-6: The addition of the
CookieAssistantimport is necessary for the updated constructor to function correctly and aligns with the PR's objectives.CHANGELOG.md (2)
- 9-11: The change log entry succinctly captures the essence of the refactoring done to optimize cookie management. It's clear and directly related to the PR objectives, ensuring readers understand the improvements made in version 1.5.0.
- 6-6: The repeated word "Added" in the sentence might be intentional due to the format of the changelog, where each entry under a version starts with "Added" or "Changed" to denote the nature of the change. If this repetition is stylistically consistent with the rest of the document, it can be considered acceptable.
tests/client/chain/grpc/test_chain_grpc_distribution_api.py (1)
- 38-38: The refactoring to use
_api_instancefor creating API instances simplifies the test setup and improves readability. This is a good practice as it reduces code duplication and centralizes the creation logic, making future changes easier to manage.pyinjective/client/indexer/grpc/indexer_grpc_explorer_api.py (1)
- 15-17: Replacing the
metadata_providerwithcookie_assistantin theIndexerGrpcExplorerApiconstructor and updating theGrpcApiRequestAssistantinitialization accordingly is a significant improvement. It aligns with the PR's goal of introducing a more generic cookie assistant, enhancing flexibility and maintainability.pyinjective/core/network.py (4)
- 42-42: The method
_check_cookie_expirationinBareMetalLoadBalancedCookieAssistantchecks for cookie expiration but is not covered by tests. Ensure that this logic is thoroughly tested, especially since cookie expiration can significantly impact functionality.- 167-169: The
BareMetalLoadBalancedCookieAssistantinstances used in thetestnetconfiguration of theNetworkclass are not covered by tests. It's crucial to test these configurations to ensure that the cookie assistant behaves as expected in different network environments.- 213-215: Similar to the
testnetconfiguration, theBareMetalLoadBalancedCookieAssistantinstances used in themainnetconfiguration of theNetworkclass are not covered by tests. Testing these configurations is essential for ensuring reliable cookie management in production environments.- 268-270: The defaulting to
DisabledCookieAssistantin thecustomconfiguration of theNetworkclass when no cookie assistant is provided is not covered by tests. Testing this fallback behavior is important for ensuring that the system behaves correctly when custom configurations are used.tests/core/tx/grpc/test_tendermint_grpc_api.py (2)
- 358-366: The
_api_instancemethod is a significant improvement in terms of code reuse and maintainability. By centralizing the creation ofTendermintGrpcApiinstances, the test setup becomes cleaner and more consistent. This change aligns well with the DRY (Don't Repeat Yourself) principle, reducing the potential for errors in repetitive setup code.- 69-69: The refactoring to use the
_api_instancemethod in test methods is correctly implemented. Each test method now calls_api_instancewith the appropriateservicerargument, simplifying the instantiation ofTendermintGrpcApi. This change enhances the readability and maintainability of the test code.Also applies to: 121-121, 149-149, 183-183, 221-221, 264-264, 328-328
pyinjective/client/indexer/grpc/indexer_grpc_derivative_api.py (1)
- 15-17: The replacement of
metadata_providerwithcookie_assistantin the constructor ofIndexerGrpcDerivativeApiand the subsequent use ofcookie_assistantfor initializingGrpcApiRequestAssistantis a positive change. It aligns with the PR's objective to introduce a more generic cookie assistant, enhancing the flexibility and applicability of cookie management across different environments. This change also simplifies the code by removing the dependency on a specific metadata provider.tests/client/indexer/grpc/test_indexer_grpc_account_api.py (2)
- 363-371: The introduction of the
_api_instancemethod in the test suite forIndexerGrpcAccountApiis a commendable change. It centralizes the setup logic for creatingIndexerGrpcAccountApiinstances, making the test code cleaner and more maintainable. This approach adheres to the DRY principle and simplifies the instantiation process across different test methods.- 37-37: The usage of the
_api_instancemethod across various test methods is correctly implemented. Each test method now utilizes this centralized method for creatingIndexerGrpcAccountApiinstances, significantly improving the readability and maintainability of the test code. This change ensures consistency in the setup process across tests and reduces the potential for setup errors.Also applies to: 83-83, 119-119, 147-147, 187-187, 232-232, 276-276, 305-305, 343-343
tests/client/chain/grpc/test_chain_grpc_wasm_api.py (1)
- 460-468: The
_api_instancemethod successfully encapsulates the creation ofChainGrpcWasmApiinstances, making the test setup more concise and maintainable. This is a good practice as it reduces code duplication and centralizes the configuration for creating test instances of the API. However, it's important to ensure that this method is flexible enough to accommodate different testing scenarios that might require variations in the setup.tests/client/chain/grpc/test_chain_grpc_bank_api.py (1)
- 444-452: The introduction of the
_api_instancemethod intest_chain_grpc_bank_api.pyis a positive change, enhancing the maintainability and readability of the test code by centralizing the instantiation logic ofChainGrpcBankApiobjects. This approach reduces code duplication and provides a single point of modification for the setup process, which is beneficial for future changes or enhancements.tests/client/chain/stream_grpc/test_chain_grpc_chain_stream.py (2)
- 197-197: The introduction of the
_api_instancemethod to create API instances with specific configurations is a good practice for simplifying test setup and making the code more maintainable.- 408-416: The
_api_instancemethod effectively encapsulates the creation ofChainGrpcChainStreaminstances, making the test setup cleaner and more reusable. However, it's important to ensure that theservicerparameter is properly documented to clarify its role and usage within this context.Consider adding a docstring to the
_api_instancemethod to explain the parameters and the method's purpose, enhancing code readability and maintainability.pyinjective/client/chain/grpc/chain_grpc_exchange_api.py (2)
- 15-15: Replacing the
metadata_providerparameter withcookie_assistantin the constructor ofChainGrpcExchangeApialigns with the PR's objective to introduce a more generic cookie management mechanism. This change enhances the flexibility of cookie handling across different environments.- 17-17: The update to use
cookie_assistantinstead ofmetadata_providerin theGrpcApiRequestAssistantinitialization is consistent with the changes made in the constructor. This ensures that the cookie management logic is centralized and more generic, which is beneficial for maintainability and adaptability to various use cases.tests/client/indexer/stream_grpc/test_indexer_grpc_spot_stream.py (1)
- 67-67: The introduction of the
_api_instancemethod across various test functions is a significant improvement in terms of code maintainability and readability. By centralizing the instantiation logic ofIndexerGrpcSpotStream, the changes contribute to reducing code duplication and making future modifications easier. This aligns well with the PR's objective of refactoring and code simplification.Also applies to: 158-158, 242-242, 324-324, 416-416, 509-509, 596-596
tests/client/indexer/grpc/test_indexer_grpc_spot_api.py (1)
- 851-859: The introduction of the
_api_instancemethod is a significant improvement in terms of code reusability and readability. By centralizing the creation ofIndexerGrpcSpotApiinstances, the test setup becomes more streamlined and easier to maintain. This change aligns well with the DRY (Don't Repeat Yourself) principle, reducing duplication and potential errors in instantiating the API across different test cases.tests/client/indexer/grpc/test_indexer_grpc_explorer_api.py (1)
- 1505-1513: The introduction of the
_api_instancemethod is a significant improvement in terms of code reusability and readability. By centralizing the creation ofIndexerGrpcExplorerApiinstances, the test setup becomes more streamlined and easier to maintain. This change aligns well with the PR objectives of refactoring for simplification and readability.tests/client/chain/grpc/test_chain_grpc_exchange_api.py (4)
- 2354-2362: The
_api_instancemethod is a significant addition to the test suite, centralizing the creation ofChainGrpcExchangeApiinstances. This change enhances the maintainability and readability of the test code by abstracting common setup logic into a reusable method. However, there are a few points to consider for further improvement:
- Hardcoded Network Configuration: The network configuration is hardcoded to
Network.devnet(). For broader test applicability, consider parameterizing the network configuration or retrieving it from a configuration file or environment variable.- Insecure Channel Warning: The use of
grpc.aio.insecure_channelis noted. Ensure that this is appropriate for the test environment and that any security implications are understood, especially if tests are run against production or sensitive environments in the future.- Cookie Assistant: The
DisabledCookieAssistantis used here, which aligns with the PR's objective to optimize cookie handling. Just ensure that this setup accurately reflects the intended use cases for these tests.Overall, this method significantly contributes to the test suite's organization and clarity.
Consider parameterizing the network configuration and reviewing the use of insecure channels in test environments.
- 62-62: The use of the
_api_instancemethod to create an instance ofChainGrpcExchangeApiis a good practice, as it centralizes the API instance creation logic, making the tests cleaner and more maintainable. This pattern is consistently applied across all test methods, which is commendable.- 62-62: In each test method, the
exchange_servicerfixture is used to mock responses for the API calls. This approach allows for controlled testing of the API client's behavior under various conditions. It's a solid testing strategy that ensures the client logic is correctly handling the mocked responses.- 62-62: Each test method correctly awaits the asynchronous API call and performs assertions on the returned data. This proper handling of asynchronous calls and the use of assertions to validate the expected outcomes are key to effective testing of async APIs.
pyinjective/async_client.py (2)
- 169-268: The changes replacing lambda functions with direct method calls for setting metadata providers in various API initializations and utilizing a
cookie_assistantattribute from thenetworkobject are consistent with the PR objectives. This simplifies the code structure and improves readability by centralizing metadata retrieval logic.- 1787-1787: The use of
metadatain streaming methods, such asstream_spot_markets, is consistent with the changes made in API initializations. However, ensure that themetadatais correctly applied in all streaming methods and that there's proper error handling for streaming failures.
Comment on lines
+654
to
+662
| def _api_instance(self, servicer): | ||
| network = Network.devnet() | ||
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | ||
| cookie_assistant = DisabledCookieAssistant() | ||
|
|
||
| api = IndexerGrpcSpotStream(channel=channel, cookie_assistant=cookie_assistant) | ||
| api._stub = servicer | ||
|
|
||
| return api |
Contributor
There was a problem hiding this comment.
The _api_instance method effectively encapsulates the creation of IndexerGrpcSpotStream instances, including the setup of the gRPC channel and the integration of the DisabledCookieAssistant. This method enhances the test code's modularity and maintainability. However, consider adding a docstring to this method to improve code documentation, explaining its purpose and the parameters it accepts.
654 def _api_instance(self, servicer):
+ """
+ Creates an instance of IndexerGrpcSpotStream for testing.
+
+ Args:
+ servicer: The gRPC servicer to be used by the IndexerGrpcSpotStream instance.
+
+ Returns:
+ An initialized instance of IndexerGrpcSpotStream.
+ """
655 network = Network.devnet()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.
Suggested change
| def _api_instance(self, servicer): | |
| network = Network.devnet() | |
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | |
| cookie_assistant = DisabledCookieAssistant() | |
| api = IndexerGrpcSpotStream(channel=channel, cookie_assistant=cookie_assistant) | |
| api._stub = servicer | |
| return api | |
| def _api_instance(self, servicer): | |
| """ | |
| Creates an instance of IndexerGrpcSpotStream for testing. | |
| Args: | |
| servicer: The gRPC servicer to be used by the IndexerGrpcSpotStream instance. | |
| Returns: | |
| An initialized instance of IndexerGrpcSpotStream. | |
| """ | |
| network = Network.devnet() | |
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | |
| cookie_assistant = DisabledCookieAssistant() | |
| api = IndexerGrpcSpotStream(channel=channel, cookie_assistant=cookie_assistant) | |
| api._stub = servicer | |
| return api |
Comment on lines
+779
to
+787
| def _api_instance(self, servicer): | ||
| network = Network.devnet() | ||
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | ||
| cookie_assistant = DisabledCookieAssistant() | ||
|
|
||
| api = IndexerGrpcDerivativeStream(channel=channel, cookie_assistant=cookie_assistant) | ||
| api._stub = servicer | ||
|
|
||
| return api |
Contributor
There was a problem hiding this comment.
The introduction of the _api_instance method is a positive change, as it centralizes the creation of IndexerGrpcDerivativeStream instances, making the test setup more concise and maintainable. However, there are a few points to consider for further improvement:
- Hardcoded
DisabledCookieAssistant: The method usesDisabledCookieAssistantdirectly. If tests require different cookie assistants in the future, consider making the cookie assistant a parameter of the_api_instancemethod to increase flexibility. - Direct Stub Assignment: Directly setting
api._stubtoservicerworks but bypasses encapsulation. If possible, modify theIndexerGrpcDerivativeStreamclass to accept a servicer in its constructor or through a setter method to maintain encapsulation.
- def _api_instance(self, servicer):
+ def _api_instance(self, servicer, cookie_assistant=DisabledCookieAssistant()):
network = Network.devnet()
channel = grpc.aio.insecure_channel(network.grpc_endpoint)
- cookie_assistant = DisabledCookieAssistant()
api = IndexerGrpcDerivativeStream(channel=channel, cookie_assistant=cookie_assistant)
+ api.set_servicer(servicer) # Assuming set_servicer is implemented
- api._stub = servicer
return apiCommittable 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.
Suggested change
| def _api_instance(self, servicer): | |
| network = Network.devnet() | |
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | |
| cookie_assistant = DisabledCookieAssistant() | |
| api = IndexerGrpcDerivativeStream(channel=channel, cookie_assistant=cookie_assistant) | |
| api._stub = servicer | |
| return api | |
| def _api_instance(self, servicer, cookie_assistant=DisabledCookieAssistant()): | |
| network = Network.devnet() | |
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | |
| api = IndexerGrpcDerivativeStream(channel=channel, cookie_assistant=cookie_assistant) | |
| api.set_servicer(servicer) # Assuming set_servicer is implemented | |
| return api |
Comment on lines
+1335
to
+1343
| def _api_instance(self, servicer): | ||
| network = Network.devnet() | ||
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | ||
| cookie_assistant = DisabledCookieAssistant() | ||
|
|
||
| api = IndexerGrpcDerivativeApi(channel=channel, cookie_assistant=cookie_assistant) | ||
| api._stub = servicer | ||
|
|
||
| return api |
Contributor
There was a problem hiding this comment.
The _api_instance method centralizes the creation of IndexerGrpcDerivativeApi instances with a custom servicer, which is a significant improvement in terms of code maintainability and readability. However, there are a few points to consider for further enhancement:
-
Hardcoded Network Configuration: The network is hardcoded to
Network.devnet(). Consider parameterizing the network or deriving it from a configuration to enhance flexibility and facilitate testing across different environments. -
Insecure Channel Creation: The method uses
grpc.aio.insecure_channelfor channel creation. Ensure that this aligns with the security requirements of your application. For production environments or sensitive data, consider using secure channels with appropriate authentication and encryption mechanisms. -
Cookie Assistant Instantiation: The method instantiates a
DisabledCookieAssistantdirectly within the function. If different tests require different cookie assistant behaviors, consider parameterizing this aspect as well. -
Stub Assignment: Directly assigning to
api._stubbypasses encapsulation principles. If possible, modify theIndexerGrpcDerivativeApiclass to allow stub injection through its constructor or a dedicated method to maintain encapsulation.
- def _api_instance(self, servicer):
+ def _api_instance(self, servicer, network=Network.devnet(), cookie_assistant=DisabledCookieAssistant()):
network = Network.devnet()
channel = grpc.aio.insecure_channel(network.grpc_endpoint)
cookie_assistant = DisabledCookieAssistant()
api = IndexerGrpcDerivativeApi(channel=channel, cookie_assistant=cookie_assistant)
- api._stub = servicer
+ api.set_stub(servicer) # Assuming set_stub is implemented in IndexerGrpcDerivativeApi
return apiConsider implementing the suggested changes to improve the method's flexibility and adherence to best practices.
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.
Suggested change
| def _api_instance(self, servicer): | |
| network = Network.devnet() | |
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | |
| cookie_assistant = DisabledCookieAssistant() | |
| api = IndexerGrpcDerivativeApi(channel=channel, cookie_assistant=cookie_assistant) | |
| api._stub = servicer | |
| return api | |
| def _api_instance(self, servicer, network=Network.devnet(), cookie_assistant=DisabledCookieAssistant()): | |
| network = Network.devnet() | |
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | |
| cookie_assistant = DisabledCookieAssistant() | |
| api = IndexerGrpcDerivativeApi(channel=channel, cookie_assistant=cookie_assistant) | |
| api.set_stub(servicer) # Assuming set_stub is implemented in IndexerGrpcDerivativeApi | |
| return api |
Comment on lines
6
to
14
| ### Added | ||
| - Added support for all queries in the chain 'tendermint' module | ||
|
|
||
| ### Changed | ||
| - Refactored cookies management logic to use all gRPC calls' responses to update the current cookies | ||
|
|
||
| ## [1.4.1] - 2024-03-12 | ||
| ### Changed | ||
| - Updates example scripts that were still using deprecated methods |
Contributor
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [51-51]
The term "low level API components" is correctly used in the context of software development. However, when used as a compound adjective before a noun, it's typically hyphenated. Consider revising for clarity.
- Added low level API components for all modules
+ Added low-level API components for all modules|
|
||
| try: | ||
| metadata = await self.network.chain_metadata(metadata_query_provider=self._chain_cookie_metadata_requestor) | ||
| metadata = self.network.chain_cookie_assistant.metadata() |
Contributor
There was a problem hiding this comment.
Consider adding explicit error handling for the fetch_account method to improve robustness and provide more informative error messages to the caller.
try:
metadata = self.network.chain_cookie_assistant.metadata()
account_any = (
await self.stubAuth.Account(auth_query.QueryAccountRequest(address=address), metadata=metadata)
).account
account = account_pb2.EthAccount()
if account_any.Is(account.DESCRIPTOR):
account_any.Unpack(account)
self.number = int(account.base_account.account_number)
self.sequence = int(account.base_account.sequence)
+except grpc.RpcError as e:
+ LoggerProvider().logger_for_class(logging_class=self.__class__).error(f"RPC error: {e}")
+ raise
except Exception as e:
LoggerProvider().logger_for_class(logging_class=self.__class__).debug(
f"error while fetching sequence and number {e}"
)
return NoneCommittable 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.
Suggested change
| metadata = self.network.chain_cookie_assistant.metadata() | |
| try: | |
| metadata = self.network.chain_cookie_assistant.metadata() | |
| account_any = ( | |
| await self.stubAuth.Account(auth_query.QueryAccountRequest(address=address), metadata=metadata) | |
| ).account | |
| account = account_pb2.EthAccount() | |
| if account_any.Is(account.DESCRIPTOR): | |
| account_any.Unpack(account) | |
| self.number = int(account.base_account.account_number) | |
| self.sequence = int(account.base_account.sequence) | |
| except grpc.RpcError as e: | |
| LoggerProvider().logger_for_class(logging_class=self.__class__).error(f"RPC error: {e}") | |
| raise | |
| except Exception as e: | |
| LoggerProvider().logger_for_class(logging_class=self.__class__).debug( | |
| f"error while fetching sequence and number {e}" | |
| ) | |
| return None |
Comment on lines
+297
to
+305
| def _api_instance(self, servicer): | ||
| network = Network.devnet() | ||
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | ||
| cookie_assistant = DisabledCookieAssistant() | ||
|
|
||
| api = ChainGrpcDistributionApi(channel=channel, cookie_assistant=cookie_assistant) | ||
| api._stub = servicer | ||
|
|
||
| return api |
Contributor
There was a problem hiding this comment.
The _api_instance method centralizes the creation of ChainGrpcDistributionApi instances with a DisabledCookieAssistant. This is a positive change for maintainability and readability. However, consider adding a brief docstring to this method to explain its purpose and parameters for future maintainability.
+ def _api_instance(self, servicer):
+ """
+ Creates an instance of ChainGrpcDistributionApi with a DisabledCookieAssistant.
+
+ :param servicer: The gRPC servicer to be used by the API instance.
+ :return: An instance of ChainGrpcDistributionApi.
+ """
network = Network.devnet()
channel = grpc.aio.insecure_channel(network.grpc_endpoint)
cookie_assistant = DisabledCookieAssistant()
api = ChainGrpcDistributionApi(channel=channel, cookie_assistant=cookie_assistant)
api._stub = servicer
return apiCommittable 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.
Suggested change
| def _api_instance(self, servicer): | |
| network = Network.devnet() | |
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | |
| cookie_assistant = DisabledCookieAssistant() | |
| api = ChainGrpcDistributionApi(channel=channel, cookie_assistant=cookie_assistant) | |
| api._stub = servicer | |
| return api | |
| def _api_instance(self, servicer): | |
| """ | |
| Creates an instance of ChainGrpcDistributionApi with a DisabledCookieAssistant. | |
| :param servicer: The gRPC servicer to be used by the API instance. | |
| :return: An instance of ChainGrpcDistributionApi. | |
| """ | |
| network = Network.devnet() | |
| channel = grpc.aio.insecure_channel(network.grpc_endpoint) | |
| cookie_assistant = DisabledCookieAssistant() | |
| api = ChainGrpcDistributionApi(channel=channel, cookie_assistant=cookie_assistant) | |
| api._stub = servicer | |
| return api |
Comment on lines
+15
to
+18
| def __init__(self, channel: Channel, cookie_assistant: CookieAssistant): | ||
| self._query_stub = token_factory_query_grpc.QueryStub(channel) | ||
| self._tx_stub = token_factory_tx_grpc.MsgStub(channel) | ||
| self._assistant = GrpcApiRequestAssistant(metadata_provider=metadata_provider) | ||
| self._assistant = GrpcApiRequestAssistant(cookie_assistant=cookie_assistant) |
Contributor
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process aimed to identify instances of ChainGrpcTokenFactoryApi being instantiated without passing the cookie_assistant parameter, as required by the updated __init__ method signature. The initial script found one such instance in pyinjective/async_client.py. A follow-up script was executed to search for any additional instances that might have been missed due to syntax variations or being located in different files, excluding the already identified instance in async_client.py. The follow-up script did not produce any output, indicating that no additional instances were found.
Given this information, it can be concluded that the only instance requiring modification to comply with the updated __init__ method signature of ChainGrpcTokenFactoryApi is the one found in pyinjective/async_client.py. This confirms the validity of the original review comment, highlighting the need to ensure that all instances of ChainGrpcTokenFactoryApi throughout the codebase are updated to pass the CookieAssistant instance.
Based on the verification process and the results obtained, the appropriate tag for this response is , as the review comment is validated by the evidence from the codebase, identifying a specific issue that needs to be addressed.
Analysis chain
The __init__ method of ChainGrpcTokenFactoryApi has been updated to use cookie_assistant instead of metadata_provider. This change aligns with the PR's objective to optimize cookie handling. Ensure that all instances of ChainGrpcTokenFactoryApi throughout the codebase are updated to pass the CookieAssistant instance.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for instances of ChainGrpcTokenFactoryApi being instantiated without cookie_assistant.
rg --type py "ChainGrpcTokenFactoryApi\(" | grep -v "cookie_assistant"
Length of output: 157
Script:
#!/bin/bash
# View the instantiation of ChainGrpcTokenFactoryApi in pyinjective/async_client.py
fd async_client.py --exec cat {}
Length of output: 145365
Script:
#!/bin/bash
# Perform a more nuanced search for ChainGrpcTokenFactoryApi instantiation to catch any instances missed due to syntax variations
rg --type py "ChainGrpcTokenFactoryApi\(" | grep -v "cookie_assistant" | grep -v "async_client.py"
Length of output: 96
Contributor
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (1)
- tests/core/test_network.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/core/test_network.py
Contributor
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (1)
- tests/core/test_network.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/core/test_network.py
Contributor
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (1)
- tests/core/test_network.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/core/test_network.py
nicolasbaum
reviewed
Mar 26, 2024
| def _check_chain_cookie_expiration(self): | ||
| if self._is_cookie_expired(cookie_data=self._chain_cookie): | ||
| self._chain_cookie = None | ||
| for key in self._expiration_time_keys_sequence[:-1]: |
There was a problem hiding this comment.
Why are you skipping last item in this iteration?
Collaborator
Author
There was a problem hiding this comment.
The variable _expiration_time_keys_sequence has all the keys that should be used to access the cookie in sequence (the cookie can be in a nested dictionary, to explain it better). So we use the list of keys without the last element just to keep going into the nested dictionaries (like a Mamushka of dictionaries), and the last element to access the cookie value
nicolasbaum
reviewed
Mar 26, 2024
| if self._is_cookie_expired(cookie_data=self._exchange_cookie): | ||
| self._exchange_cookie = None | ||
| expiration_data: Optional[str] = cookie_map.get(self._expiration_time_keys_sequence[-1], None) | ||
| if expiration_data is not None: |
There was a problem hiding this comment.
So if key is missing in the cookie_map then it's not expired. Is this confirmed to be this way by default?
Collaborator
Author
There was a problem hiding this comment.
Yes, correct. I have tested it with the current bare metal load balanced servers, and the k8s load balanced servers. If cookie is not renewed, then the response from the server does not include the info
nicolasbaum
reviewed
Mar 26, 2024
nicolasbaum
left a comment
There was a problem hiding this comment.
Understood the applied change and agree with implementation. Thanks
nicolasbaum
approved these changes
Mar 26, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://injective-labs.atlassian.net/browse/CHAIN-20?atlOrigin=eyJpIjoiZDFlNmFlOWJjNjdkNGI3NjgwN2QwOTcyZDI3MDA2NzUiLCJwIjoiaiJ9
Summary by CodeRabbit