test common::subgraph #1742
test common::subgraph #1742
Conversation
|
Warning Rate limit exceeded@0xgleb has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 10 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 (13)
WalkthroughThe changes convert the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant SubgraphArgs
participant OrderbookSubgraphClient
Caller->>SubgraphArgs: to_subgraph_client()
activate SubgraphArgs
SubgraphArgs-->>Caller: Result<OrderbookSubgraphClient, ParseError>
deactivate SubgraphArgs
Caller->>OrderbookSubgraphClient: (subsequent async calls, e.g., order_detail().await)
Previous Flow (for comparison): sequenceDiagram
participant Caller
participant SubgraphArgs
participant OrderbookSubgraphClient
Caller->>SubgraphArgs: to_subgraph_client().await
activate SubgraphArgs
SubgraphArgs-->>Caller: Result<OrderbookSubgraphClient, ParseError>
deactivate SubgraphArgs
Caller->>OrderbookSubgraphClient: (subsequent async calls, e.g., order_detail().await)
✨ 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 (
|
0xgleb
force-pushed
the
test-common-subgraph
branch
from
7844b59 to
3a5a42a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
crates/cli/src/commands/order/detail.rs(1 hunks)crates/cli/src/commands/order/list.rs(2 hunks)crates/cli/src/commands/order/remove.rs(1 hunks)crates/cli/src/commands/trade/detail.rs(1 hunks)crates/cli/src/commands/trade/list.rs(2 hunks)crates/cli/src/commands/vault/detail.rs(1 hunks)crates/cli/src/commands/vault/list.rs(2 hunks)crates/cli/src/commands/vault/list_balance_changes.rs(2 hunks)crates/common/src/subgraph.rs(1 hunks)crates/subgraph/src/orderbook_client.rs(2 hunks)tauri-app/src-tauri/src/commands/order.rs(1 hunks)tauri-app/src-tauri/src/commands/order_take.rs(1 hunks)tauri-app/src-tauri/src/commands/vault.rs(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
crates/common/src/subgraph.rs (4)
crates/subgraph/src/orderbook_client.rs (2)
new(72-74)url(76-78)crates/cli/src/commands/order/list.rs (2)
subgraph_args(32-38)subgraph_args(44-53)crates/cli/src/commands/trade/list.rs (2)
subgraph_args(32-38)subgraph_args(44-55)crates/cli/src/commands/vault/list_balance_changes.rs (2)
subgraph_args(32-38)subgraph_args(44-53)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: git-clean
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: Deploy-Preview
- GitHub Check: test
- GitHub Check: test
- GitHub Check: build-tauri (ubuntu-22.04, true)
🔇 Additional comments (18)
crates/cli/src/commands/order/detail.rs (1)
19-20: Successfully converted subgraph client creation to synchronousThe change properly removes the
.awaitcall when creating the subgraph client, reflecting thatto_subgraph_clienthas been refactored from an asynchronous to a synchronous function. The error handling using the?operator is maintained correctly.tauri-app/src-tauri/src/commands/order_take.rs (1)
16-17: Correct conversion to synchronous client creationThe change properly removes the
.awaitcall when creating the subgraph client, correctly handling the error propagation with the?operator while preserving the asynchronousawaitfor the subsequentorder_trades_list_allcall.crates/cli/src/commands/order/remove.rs (1)
27-28: Client creation correctly converted to synchronousThe change appropriately removes the
.awaitcall when creating the subgraph client, maintaining proper error propagation with the?operator. The function flow remains the same, with only the subgraph client creation becoming synchronous.tauri-app/src-tauri/src/commands/order.rs (3)
19-20: Properly updated client creation to synchronousThe
.awaitcall has been correctly removed when creating the subgraph client, maintaining error handling with the?operator. The subsequentorders_list_allcall remains properly asynchronous.
62-73: Verify error handling in synchronous client creationI notice that this section doesn't have any changed lines, but it's using the
to_subgraph_client()method without.await. This section appears to have already been updated previously to use the synchronous version, handling errors with appropriate toast messages, which is consistent with the changes being made in this PR.
114-125: Consistent pattern for synchronous client creationSimilar to the previous section, this code is already using the synchronous version of
to_subgraph_client()without.await, with proper error handling and user notifications. This maintains consistency with the changes being made in this PR.crates/cli/src/commands/vault/detail.rs (1)
20-20: Successfully converted async client creation to synchronous.The removal of
.awaitfromto_subgraph_client()correctly implements the PR objective of removing unnecessary async operations. This is a good simplification that maintains the same functionality while potentially improving performance by reducing async overhead.crates/subgraph/src/orderbook_client.rs (2)
59-59: Appropriately added Debug trait derivation.Adding the
Debugtrait toOrderbookSubgraphClientimproves diagnostics and testing capabilities, which aligns well with the PR's goal of enhancing testability for the subgraph module.
76-78: Good addition of URL accessor method.This accessor follows Rust best practices by:
- Returning a reference instead of cloning the URL
- Providing better visibility for testing
- Supporting the PR's goal of improved testability
The method will be useful for assertions in the new tests being added in this PR.
crates/cli/src/commands/trade/detail.rs (1)
22-22: Successfully converted async client creation to synchronous.The removal of
.awaitfromto_subgraph_client()correctly implements the PR objective. This change is consistent with the pattern applied across other command modules.crates/cli/src/commands/vault/list.rs (2)
34-35: Successfully converted async client creation to synchronous in vaults_list_all path.The removal of
.awaitfromto_subgraph_client()correctly implements the PR objective while maintaining proper error handling with the?operator.
48-49: Successfully converted async client creation to synchronous in vaults_list path.The second instance of this change maintains consistency in both code paths (CSV and non-CSV output modes). This systematically applies the pattern throughout the file.
crates/cli/src/commands/order/list.rs (1)
33-33: Correctly converted asynchronous call to synchronousThe conversion of
.to_subgraph_client().await?to.to_subgraph_client()?aligns with the PR objective of removing unnecessary async from theto_subgraph_clientfunction. This change maintains proper error handling with the?operator while making the client creation synchronous.Also applies to: 45-45
tauri-app/src-tauri/src/commands/vault.rs (1)
23-23: Properly removed .await from subgraph client creationThe synchronous call to
to_subgraph_client()with direct error propagation using?is consistent with the changes across the codebase. This simplifies the code while maintaining the same error handling pattern.Also applies to: 44-44
crates/cli/src/commands/trade/list.rs (1)
33-33: Correctly converted to synchronous client creationThe changes appropriately remove the async/await pattern from the client creation while preserving the asynchronous nature of the subsequent subgraph operations. This is consistent with the overall objective of making
to_subgraph_client()synchronous.Also applies to: 45-45
crates/cli/src/commands/vault/list_balance_changes.rs (1)
33-33: Successfully converted to synchronous client creationThe removal of
.awaitfrom theto_subgraph_client()calls correctly implements the PR objective. The error handling approach with?is maintained, ensuring that any client creation errors are still properly propagated while simplifying the code.Also applies to: 45-45
crates/common/src/subgraph.rs (2)
11-11: Clean simplification from async to sync functionGood change to remove the unnecessary
asynckeyword from this method. The function doesn't perform any I/O operations - it only parses a URL string and constructs a client - so making it synchronous is more appropriate. This will reduce overhead and improve clarity for callers.
16-52: Great test coverage for the subgraph client creationThe added tests comprehensively verify both successful and error cases:
- Tests valid URL construction and verification
- Tests multiple error scenarios including missing scheme, empty host, empty string, and malformed URL
- Properly asserts the specific error types returned
This significantly improves the reliability of the codebase by ensuring URL parsing behaves as expected across various inputs.
0xgleb
force-pushed
the
test-common-subgraph
branch
from
3a5a42a to
2870984
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Motivation
common::subgraphto_subgraph_clientSolution
to_subgraph_clientnot asyncChecks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Refactor
Tests