Implement builder API for client #91
Conversation
WalkthroughReplace ClientConfig-based construction with a fluent Client::builder() API; move client internal state into an inner Arc with Changes
Sequence Diagram(s)sequenceDiagram
participant User as Example/Test
participant Builder as ClientBuilder
participant Config as ClientConfig
participant Validator as ClientConfig::validate
participant Client as Client
Note over User,Builder: Builder-based client construction (new flow)
User->>Builder: Client::builder()
User->>Builder: .url(...) /.chain_id(...) /.api_token(...) / ...
User->>Builder: .build()
Builder->>Config: assemble ClientConfig
Builder->>Validator: Config::validate()
Validator-->>Builder: Ok / Err
alt Ok
Builder->>Client: Client::new(validated config)
Client-->>User: Ok(Client)
else Err
Validator-->>User: validation error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.cargo/config.toml(1 hunks)examples/all_erc20/src/main.rs(2 hunks)examples/call_decode_output/src/main.rs(2 hunks)examples/call_watch/src/main.rs(2 hunks)examples/height_stream/src/main.rs(1 hunks)examples/reverse_wallet/src/main.rs(1 hunks)examples/wallet/src/main.rs(2 hunks)examples/watch/src/main.rs(2 hunks)hypersync-client/Cargo.toml(1 hunks)hypersync-client/src/config.rs(2 hunks)hypersync-client/src/lib.rs(13 hunks)hypersync-client/tests/api_test.rs(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
examples/wallet/src/main.rs (1)
hypersync-client/src/lib.rs (1)
builder(95-97)
examples/all_erc20/src/main.rs (1)
hypersync-client/src/lib.rs (1)
builder(95-97)
examples/call_watch/src/main.rs (1)
hypersync-client/src/lib.rs (1)
builder(95-97)
examples/reverse_wallet/src/main.rs (3)
examples/wallet/src/main.rs (1)
main(18-126)examples/watch/src/main.rs (1)
main(13-86)hypersync-client/src/lib.rs (1)
builder(95-97)
examples/height_stream/src/main.rs (2)
examples/wallet/src/main.rs (1)
main(18-126)hypersync-client/src/lib.rs (3)
new(76-81)new(731-733)builder(95-97)
examples/call_decode_output/src/main.rs (1)
hypersync-client/src/lib.rs (1)
builder(95-97)
hypersync-client/tests/api_test.rs (1)
hypersync-client/src/lib.rs (1)
builder(95-97)
hypersync-client/src/lib.rs (1)
hypersync-client/src/config.rs (1)
default(57-68)
hypersync-client/src/config.rs (1)
hypersync-client/src/lib.rs (2)
url(705-707)url(775-778)
examples/watch/src/main.rs (1)
hypersync-client/src/lib.rs (1)
builder(95-97)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test_release
- GitHub Check: test_dev
🔇 Additional comments (6)
hypersync-client/Cargo.toml (1)
55-55: Dependency move is justified —uuidis used in runtime code.The verification confirms that
uuidis actively used in the main library code athypersync-client/src/config.rs:89for validating bearer tokens duringClientConfigvalidation. This is legitimate runtime usage, not just test code, so movinguuidfrom[dev-dependencies]to[dependencies]is correct and necessary..cargo/config.toml (1)
2-3: Good call providing a dummy token.This keeps doctests functional without risking real credentials.
examples/call_watch/src/main.rs (1)
16-21: Builder migration looks solid.Chain selection and bearer token wiring align with the new API expectations.
examples/call_decode_output/src/main.rs (1)
18-23: Nice reuse of the builder chain.You carried over the retry tuning while enforcing the new auth flow.
examples/reverse_wallet/src/main.rs (1)
16-20: Example ties into the builder cleanly.The default mainnet setup now goes through the supported construction path.
examples/height_stream/src/main.rs (1)
10-15: Height stream setup handles the new builder well.Explicit URL plus
?onbuild()keeps the example robust.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/call_decode_output/src/main.rs (1)
43-43: Remove unnecessary clone.Since
stream_arrownow takes&self, the.clone()call is unnecessary.Apply this diff:
- let mut rx = client.clone().stream_arrow(query, config).await?; + let mut rx = client.stream_arrow(query, config).await?;
🧹 Nitpick comments (1)
hypersync-client/src/lib.rs (1)
965-973: Consider validating bearer_token in build().The
build()method validates that the URL is not empty, but it doesn't check ifbearer_tokenis empty. While an empty bearer token might be intentional for unauthenticated endpoints, it would be helpful to either validate it or document that it's optional.Consider adding validation:
pub fn build(self) -> Result<Client> { if self.0.url.is_empty() { anyhow::bail!( "endpoint needs to be set, try using builder.chain_id(1) or\ builder.url(\"https://eth.hypersync.xyz\") to set the endpoint" ) } if self.0.bearer_token.is_empty() { anyhow::bail!( "bearer_token needs to be set, try using builder.bearer_token(...)" ) } Client::new(self.0) }Or document in the
ClientBuilderdocstring that bearer_token can be empty for testing/unauthenticated usage if that's intended.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/all_erc20/src/main.rs(2 hunks)examples/call_decode_output/src/main.rs(2 hunks)examples/height_stream/src/main.rs(1 hunks)examples/reverse_wallet/src/main.rs(2 hunks)examples/wallet/src/main.rs(2 hunks)hypersync-client/src/lib.rs(30 hunks)hypersync-client/src/parquet_out.rs(1 hunks)hypersync-client/src/stream.rs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/wallet/src/main.rs
- examples/all_erc20/src/main.rs
- examples/reverse_wallet/src/main.rs
🧰 Additional context used
🧬 Code graph analysis (3)
examples/height_stream/src/main.rs (3)
examples/all_erc20/src/main.rs (1)
main(17-113)examples/wallet/src/main.rs (1)
main(16-123)hypersync-client/src/lib.rs (1)
builder(101-103)
examples/call_decode_output/src/main.rs (1)
hypersync-client/src/lib.rs (1)
builder(101-103)
hypersync-client/src/lib.rs (1)
hypersync-client/src/config.rs (2)
default(37-48)default(177-194)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: test_release
- GitHub Check: test_dev
🔇 Additional comments (10)
hypersync-client/src/parquet_out.rs (1)
24-29: LGTM! Signature change aligns with the new borrowing model.The change from
Arc<Client>to&Clientis appropriate and consistent with the broader refactoring. The internalclient.clone()on line 56 works correctly sinceClientimplementsClone.examples/call_decode_output/src/main.rs (1)
16-21: Builder pattern usage looks good.The migration to the builder API is clean and follows the established pattern across the codebase.
examples/height_stream/src/main.rs (1)
8-13: LGTM! Clean migration to the builder pattern.The builder usage is correct and the direct call to
stream_height()aligns with the new API design.hypersync-client/src/stream.rs (2)
28-39: LGTM! Signature changes and StreamConfig simplification.The migration from
Arc<Client>to&Clientis well-executed, and convertingStreamConfigfields fromOption<T>to concrete types with defaults reduces cognitive overhead.
420-423: LGTM! Consistent borrowing semantics.The signature change to
&crate::Clientis consistent with the broader refactoring.hypersync-client/src/lib.rs (5)
51-76: LGTM! Well-structured internal state encapsulation.The introduction of
ClientInnerwrapped inArcprovides clean separation and enables efficient cloning. Thebearer_tokenfield is now concrete (String), which simplifies the API.
101-103: LGTM! Clean builder entry point.The
builder()method provides an intuitive starting point for client configuration.
723-974: LGTM! Comprehensive and well-documented builder API.The
ClientBuilderimplementation is thorough with:
- Clear documentation for each method
- Sensible method chaining
- Good examples throughout
- Convenient
chain_id()helper that constructs the URLThe fluent API design will improve developer experience significantly.
1004-1032: LGTM! Well-structured SSE stream setup.The
get_es_stream()method correctly:
- Constructs the URL path
- Applies bearer authentication
- Configures exponential backoff
- Handles EventSource initialization
1183-1194: LGTM! Clean stream height API.The method signature using
&selfis appropriate, and spawning a background task withclient.clone()correctly handles the ownership for the async context.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
hypersync-client/tests/api_test.rs (3)
203-206: Missing api_token will cause build() to panic.The builder requires an API token, but this test omits
.api_token(...)before calling.build().unwrap(). The validation added inClientConfig::validate()will fail, causing a panic.Apply this diff:
let client = Client::builder() .url("https://base.hypersync.xyz") + .api_token(std::env::var("ENVIO_API_TOKEN").unwrap()) .build() .unwrap();
498-501: Missing api_token will cause build() to panic.Same issue: this builder call lacks
.api_token(...)and will panic during validation.Apply this diff:
let client = Arc::new( Client::builder() .url("https://mev-commit.hypersync.xyz") + .api_token(std::env::var("ENVIO_API_TOKEN").unwrap()) .build() .unwrap(), );
537-543: Missing api_token will cause build() to panic.This builder also lacks
.api_token(...)before.build().unwrap().Apply this diff:
let client = Arc::new( Client::builder() .url("http://localhost:1131") + .api_token(std::env::var("ENVIO_API_TOKEN").unwrap()) .serialization_format(SerializationFormat::CapnProto { should_cache_queries: true, }) .build() .unwrap(), );
🧹 Nitpick comments (3)
hypersync-schema/src/lib.rs (1)
71-72: Consider more explicit constant naming to avoid negation.The
!NULLABLEpattern throughout the codebase is less readable than using explicit constants. Consider one of these alternatives:Option 1 (recommended): Use two constants to avoid negation
-const NULLABLE: bool = true; +const NULLABLE: bool = true; +const REQUIRED: bool = false;Then use
REQUIREDinstead of!NULLABLEin field definitions.Option 2: Use more descriptive naming
-const NULLABLE: bool = true; +const IS_NULLABLE: bool = true; +const IS_REQUIRED: bool = false;hypersync-client/src/lib.rs (2)
1410-1418: Consider validating api_token in build().The
build()method checks for an empty URL but doesn't check ifapi_tokenis set. WhileClient::new()will catch this viavalidate(), checking earlier in the builder would provide a clearer error message at the point of misconfiguration.Consider adding:
pub fn build(self) -> Result<Client> { if self.0.url.is_empty() { anyhow::bail!( "endpoint needs to be set, try using builder.chain_id(1) or\ builder.url(\"https://eth.hypersync.xyz\") to set the endpoint" ) } + if self.0.api_token.is_empty() { + anyhow::bail!( + "api_token is required - get one from https://envio.dev/app/api-tokens" + ) + } Client::new(self.0) }
1210-1213: Document chain_id URL pattern assumption.The
chain_id()method assumes all chains are available athttps://{chain_id}.hypersync.xyz. Consider documenting which chain IDs are supported or providing a link to the list of available endpoints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.cargo/config.toml(1 hunks)examples/all_erc20/src/main.rs(2 hunks)examples/call_decode_output/src/main.rs(2 hunks)examples/call_watch/src/main.rs(2 hunks)examples/height_stream/src/main.rs(1 hunks)examples/reverse_wallet/src/main.rs(2 hunks)examples/wallet/src/main.rs(2 hunks)examples/watch/src/main.rs(2 hunks)hypersync-client/src/config.rs(4 hunks)hypersync-client/src/lib.rs(30 hunks)hypersync-client/tests/api_test.rs(4 hunks)hypersync-format/src/lib.rs(1 hunks)hypersync-net-types/src/lib.rs(1 hunks)hypersync-schema/src/lib.rs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- hypersync-net-types/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/call_decode_output/src/main.rs
- examples/wallet/src/main.rs
- .cargo/config.toml
🧰 Additional context used
🧬 Code graph analysis (8)
examples/reverse_wallet/src/main.rs (2)
examples/wallet/src/main.rs (1)
main(16-123)hypersync-client/src/lib.rs (1)
builder(188-190)
examples/watch/src/main.rs (1)
hypersync-client/src/lib.rs (1)
builder(188-190)
hypersync-client/tests/api_test.rs (1)
hypersync-client/src/lib.rs (1)
builder(188-190)
examples/call_watch/src/main.rs (1)
hypersync-client/src/lib.rs (1)
builder(188-190)
examples/height_stream/src/main.rs (2)
examples/wallet/src/main.rs (1)
main(16-123)hypersync-client/src/lib.rs (1)
builder(188-190)
examples/all_erc20/src/main.rs (1)
hypersync-client/src/lib.rs (1)
builder(188-190)
hypersync-client/src/lib.rs (2)
hypersync-client/src/stream.rs (2)
new(461-477)stream_arrow(28-213)hypersync-client/src/config.rs (2)
default(37-48)default(177-194)
hypersync-client/src/config.rs (1)
hypersync-client/src/lib.rs (2)
url(1162-1164)url(1233-1236)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test_release
- GitHub Check: test_dev
🔇 Additional comments (16)
hypersync-schema/src/lib.rs (2)
1-44: Excellent module documentation!The comprehensive documentation with examples and feature descriptions provides a clear entry point for users of this crate.
73-206: This is a pure refactoring with no semantic changes—the nullability values remain identical.The git history shows commit 00ee12c replaces literal boolean values with named constants (NULLABLE=true, !NULLABLE=false) for improved readability. Every field's nullability is unchanged:
false→!NULLABLE(both represent not nullable)true→NULLABLE(both represent nullable)There are no breaking changes to schema compatibility or consuming code.
examples/height_stream/src/main.rs (1)
8-11: LGTM! Clean builder pattern implementation.The migration to the builder pattern is well-executed, with proper error handling via
?for both the environment variable andbuild()call.examples/call_watch/src/main.rs (1)
17-21: LGTM! Good use of chain_id() convenience method.The builder pattern is correctly applied, using
chain_id(1)as a convenient shorthand for Ethereum mainnet.examples/reverse_wallet/src/main.rs (1)
14-18: LGTM! Consistent builder pattern usage.The example correctly demonstrates the builder API with proper authentication.
examples/watch/src/main.rs (1)
17-21: LGTM! Consistent example migration.Properly demonstrates the new builder API with environment-based authentication.
examples/all_erc20/src/main.rs (1)
21-28: LGTM! Demonstrates advanced builder configuration.This example correctly shows how to configure both authentication and serialization format through the builder API.
hypersync-client/src/lib.rs (4)
123-148: Good encapsulation of client state.The
ClientInnerstruct properly centralizes configuration and credentials behind an Arc, withapi_tokenas a required field.
169-222: LGTM! Proper validation before construction.The
validate()call on Line 171 ensures configuration is valid before building the client, preventing runtime errors from misconfiguration.
525-529: Consistent bearer authentication pattern.The use of
.bearer_auth(&self.inner.api_token)across all HTTP requests ensures consistent authentication handling.
17-53: The code is already properly configured—no changes needed.The example code is marked with
no_run(as shown in the snippet), which prevents it from executing as a doctest. Additionally,.cargo/config.tomlalready provides a dummyENVIO_API_TOKENfor tests. The original concern is already addressed.Likely an incorrect or invalid review comment.
hypersync-client/src/config.rs (3)
77-103: Thorough validation with helpful error messages.The
validate()method provides comprehensive checks with actionable error messages, including the link to get an API token. The UUID validation forapi_tokenis a good safeguard against configuration errors.
51-75: Reasonable default values for production use.The defaults provide a good balance: 30-second HTTP timeout, 12 retries with exponential backoff capped at 5 seconds. This should handle transient failures without excessive delays.
234-321: Comprehensive test coverage for configuration.The tests cover validation edge cases, default values, and serde round-trips, including partial JSON deserialization. This provides good confidence in the configuration behavior.
hypersync-format/src/lib.rs (2)
43-49: LGTM! Public API expansion aligns with PR objectives.The re-exports appropriately expose the core types and error handling needed for the builder API pattern being introduced in the hypersync-client crate. All key types mentioned in the documentation are included.
1-39: Documentation and example code verified as correct.The crate documentation is well-structured and comprehensive. The doctest example is syntactically valid and should compile successfully:
AddressandHashare properly exported from the crate- Both types correctly implement
std::str::FromStrfor.parse()support- The hex string formats are valid (0x + 40 hex chars for Address, 0x + 64 hex chars for Hash)
- The error type is compatible with the doctest's return type
Box<dyn std::error::Error>- Display formatting is properly implemented for the
println!statements
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
examples/wallet/src/main.rs (1)
19-23: Builder pattern implemented correctly; consider consistent error handling.The builder pattern usage is correct and properly configures the client with URL and API token. For consistency, consider using the
?operator instead ofunwrap()on line 23, since you're already propagating errors with?on line 21.Apply this diff for consistent error handling:
- .build() - .unwrap(); + .build()?;hypersync-client/src/lib.rs (1)
1410-1418: Consider validating api_token in builder.The builder validates that URL is set, but doesn't check if
api_tokenis empty. Since the token is required for authentication, consider adding validation for it as well in thebuild()method.Example validation:
pub fn build(self) -> Result<Client> { if self.0.url.is_empty() { anyhow::bail!( "endpoint needs to be set, try using builder.chain_id(1) or\ builder.url(\"https://eth.hypersync.xyz\") to set the endpoint" ) } + if self.0.api_token.is_empty() { + anyhow::bail!( + "api_token needs to be set, try using builder.api_token(std::env::var(\"ENVIO_API_TOKEN\")?) to set the token" + ) + } Client::new(self.0) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/wallet/src/main.rs(2 hunks)hypersync-client/Cargo.toml(2 hunks)hypersync-client/src/config.rs(4 hunks)hypersync-client/src/lib.rs(29 hunks)hypersync-format/src/lib.rs(1 hunks)hypersync-net-types/Cargo.toml(1 hunks)hypersync-net-types/src/lib.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- hypersync-net-types/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- hypersync-client/Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (3)
examples/wallet/src/main.rs (1)
hypersync-client/src/lib.rs (1)
builder(188-190)
hypersync-client/src/lib.rs (2)
hypersync-client/src/stream.rs (2)
new(461-477)stream_arrow(28-213)hypersync-client/src/config.rs (2)
default(37-48)default(175-192)
hypersync-client/src/config.rs (1)
hypersync-client/src/lib.rs (2)
url(1162-1164)url(1233-1236)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test_release
🔇 Additional comments (19)
hypersync-format/src/lib.rs (2)
1-39: Excellent documentation structure!The crate-level documentation is comprehensive and well-organized. It provides a clear overview of the crate's purpose, features, key types, and practical examples. The doc-test example follows Rust conventions and demonstrates the most common use cases effectively.
43-49: LGTM! Clean API surface.The public re-exports appropriately flatten the module hierarchy, making core types directly accessible from the crate root. All key types mentioned in the documentation are included, providing a coherent and user-friendly API.
hypersync-net-types/src/lib.rs (1)
1-46: LGTM! Excellent documentation improvements.The new crate-level documentation provides a clear overview of the library's purpose, features, and usage patterns. The example code demonstrates proper usage of the Query builder API.
hypersync-client/src/lib.rs (8)
1-74: LGTM! Comprehensive module documentation.The new documentation provides clear guidance on using the client, including a Quick Start example that demonstrates the builder pattern, proper authentication, and common usage patterns.
123-142: LGTM! Clean internal state encapsulation.The new
ClientInnerstruct properly encapsulates the client's internal state and will be shared viaArc. The renaming frombearer_tokentoapi_tokenis more semantically accurate.
144-222: LGTM! Well-structured client initialization.The refactoring to use
Arc<ClientInner>is clean and allows for efficient cloning. The addition ofClient::builder()provides a more ergonomic API while maintaining backward compatibility withClient::new(cfg).
268-517: LGTM! Improved method signatures and documentation.The change from
Arc<Self>to&selfis more ergonomic and idiomatic now thatClientinternally usesArc<ClientInner>. The expanded documentation with warnings about long-running queries is very helpful for users.
520-972: LGTM! Proper internal state access.All HTTP implementation methods correctly access fields through
self.inner, and the authentication properly usesself.inner.api_token. The retry logic remains intact and functional.
588-1165: LGTM! Consistent API updates.All public methods have been properly updated to work with the new internal state pattern. The documentation examples consistently demonstrate the builder pattern, and the
url()method correctly returns a reference to the inner URL.
1167-1409: LGTM! Well-designed fluent builder API.The
ClientBuilderimplementation provides a clean, fluent interface for client configuration. Each method is properly documented with examples, and the pattern follows Rust builder conventions.
1449-1639: LGTM! Proper SSE stream implementation.The SSE streaming methods correctly access internal state through
self.inner, and thestream_height()method properly clones the client (which is cheap due to the internalArc) before spawning the background task.hypersync-client/src/config.rs (8)
7-34: LGTM! Configuration structure well-designed.The conversion from optional types to concrete types with explicit defaults provides better clarity and allows the builder pattern to work naturally with validation occurring at build time rather than field-by-field.
36-49: LGTM!Default implementation properly delegates to const helper functions for consistency.
51-75: LGTM! Well-structured default helpers.Using
const fnfor default value providers is excellent for compile-time evaluation and makes the defaults clearly visible.
116-160: LGTM! StreamConfig design is well-thought-out.The distinction between fields that have sensible defaults (batch sizes, concurrency) and those that should remain optional (max_num limits) is appropriate.
162-172: LGTM!Adding
PartialEqenables the equality assertion in the tests (line 289), and the explicit#[default]onNoEncodemakes the default behavior clear.
174-230: LGTM! StreamConfig defaults are well-structured.The helper functions provide clear, compile-time constants, and the default values appear reasonable for streaming workloads.
232-319: LGTM! Comprehensive test coverage.The tests properly cover:
- All validation branches (valid config, invalid UUID, missing fields, zero timeout)
- Default value correctness
- Serde round-trip and partial deserialization behavior
77-100: I need to examine the config structure and serde setup to better understand the constraints.Now let me check the Config struct definition directly:
Review comment is accurate but concern is overstated—the double parsing has negligible performance impact in practice.
The code does parse the URL twice as the review states: once in
validate()at config.rs:83 and again during client creation at lib.rs:208. However, this is an intentional trade-off made for serde simplicity. Enabling the serde feature on the url crate is required for Url to implement Serialize and Deserialize, and storing the string representation avoids adding an extra dependency. Since URL parsing happens only during client initialization (not per-request) and completes in microseconds, the performance cost is negligible. The URL is then stored as a parsedUrlinClientInnerand cloned for path manipulation in subsequent operations, which is efficient. The current design is reasonable and doesn't warrant refactoring.
JasoonS
left a comment
JasoonS
left a comment
There was a problem hiding this comment.
Awesome, looking forward to it!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
hypersync-client/tests/api_test.rs (1)
619-628: Consider simplifying iteration limiting.The manual counter works but could potentially be replaced with a more idiomatic approach if the receiver supports limiting. However, the current implementation is clear and explicit for test purposes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hypersync-client/tests/api_test.rs(17 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
hypersync-client/tests/api_test.rs (1)
hypersync-client/src/lib.rs (3)
builder(188-190)new(169-174)new(1189-1191)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test_release
🔇 Additional comments (2)
hypersync-client/tests/api_test.rs (2)
18-22: Previous review feedback has been addressed.All client instantiations now include
.api_token(std::env::var("ENVIO_API_TOKEN").unwrap()), which resolves the validation panic issue identified in the previous review.Also applies to: 53-57, 118-124, 186-190, 217-222, 272-278, 329-335, 360-364, 389-393, 413-417, 443-447, 465-469, 493-499, 551-556
591-599: The review comment is incorrect.The
chain_id(1)call does set a URL. Thechain_id()method setsself.0.url = format!("https://{chain_id}.hypersync.xyz"), so the test's builder chain produces"https://1.hypersync.xyz"and will not fail validation. The.build().unwrap()will succeed without panic.The test is correct as written and does not require modification.
Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Refactor
Chores