Open
Conversation
Signed-off-by: teodordelibasic-db <teodor.delibasic@databricks.com>
0695a36 to
74b5211
Compare
Signed-off-by: teodordelibasic-db <teodor.delibasic@databricks.com>
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
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.
What changes are proposed in this pull request?
This PR adds a typestate
StreamBuilderAPI to the Rust SDK that replaces the existingcreate_stream*family of methods with a fluent, compile-time-safe builder.The builder enforces a strict configuration order: auth -> format -> config -> build. Invalid combinations (building without auth, building without format, calling gRPC-only setters on an Arrow stream) are compile errors, not runtime errors.
Motivation:
create_stream(TableProperties, client_id, client_secret, Option<StreamConfigurationOptions>)API is a wide parameter list where format is just a field insideStreamConfigurationOptions. Adding or removing fields is a breaking change.Changes:
New files:
rust/sdk/src/builder/stream_builder.rs— theStreamBuilder<F, A>typestate builder with format/auth marker generics, all setter methods, three format-specificbuild()impls, and 12 unit testsrust/sdk/src/builder/stream_format.rs— marker types (NoFormat,Json,CompiledProto,Arrow,NoAuth,HasAuth), sealed traits (StreamFormat,GrpcFormat), and trait implsModified:
rust/sdk/src/lib.rs— addedstream_builder()entry point onZerobusSdk, deprecated all fourcreate_stream*/create_arrow_stream*methods withsince = "1.1.0", changedworkspace_id/tls_config/get_or_create_channel_zerobus_client/ZerobusStream::new_streamtopub(crate)so the builder can access SDK internals, refactoredrecreate_streamandrecreate_arrow_streamto call internal constructors directly instead of routing through deprecated methods, fixed crate-level quick-start doc examplerust/sdk/src/builder/mod.rs— added module declarations and re-exports for stream builder and format typesrust/sdk/src/headers_provider.rs— addedNoOpHeadersProviderfor local dev / testing / sidecar proxy scenarios (used bystream_builder(...).no_auth())rust/examples/json/single/src/main.rs— migrated to builder APIrust/examples/json/batch/src/main.rs— migrated to builder APIrust/examples/proto/single/src/main.rs— migrated to builder APIrust/examples/proto/batch/src/main.rs— migrated to builder APIrust/CHANGELOG.md— added v1.1.0 release notes with new features, deprecations, and API changesrust/README.md— rewrote stream creation section, custom auth example, callbacks example, recovery example, API reference, and architecture diagram to use the new builder APIDesign decisions:
build()forcesrecord_typeto match the typestate regardless of what.options()set, as a safety belt..options(StreamConfigurationOptions)is kept as a migration bridge for users transitioning from the old API. It replaces all prior setter values and is documented as such.StreamFormat/GrpcFormatprevent external implementations, preserving forward compatibility.#[must_use]onStreamBuilderwarns if a builder is created but never built.How is this tested?
12 unit tests in
stream_builder.rs:json_oauth_builder_compiles— verifies JSON + OAuth builder chain compilescompiled_proto_headers_provider_compiles— verifies Proto + custom headers provider chain compilesconfig_setters_chain— verifies all shared and gRPC-specific setters chain correctlyoptions_replaces_config— verifies.options()replaces prior setter valuessetter_after_options_mutates_replacement— verifies setters after.options()modify the replacement configdefault_config_without_setters— verifies sensible defaults when no setters are calledoptions_cannot_override_record_type— verifies.options()with wrongrecord_typeis overridden at build timeno_auth_shortcut_compiles— verifiesno_auth()convenience compilesdebug_impl_works— verifies Debug output includes builder statenoop_headers_provider_returns_empty— verifiesNoOpHeadersProviderreturns empty headersresolve_headers_provider_with_custom_provider— verifies custom provider resolution returns expected headersresolve_headers_provider_with_oauth— verifies OAuth provider resolution constructs without panicAdditionally:
cargo test -p databricks-zerobus-ingest-sdkpasses (81 tests, including all pre-existing tests)cargo check.json()before auth, calling.build()onNoFormat) fail to compile with missing-method errorsFollow-up items
These are scoped out of this PR and tracked for subsequent work.
Introduce a shared internal stream-open abstraction (e.g.,
StreamOpenRequestorResolvedStreamConfig) that all stream creation paths target: the builder, the deprecated methods, the recreate methods, and the FFI/JNI/PyO3/NAPI wrapper bridges. This is the critical architectural step before removing deprecated methods in 2.0, because without it wrapper SDKs will need a second migration.Add integration tests for builder-based stream creation in
rust/tests/srcagainst the mock gRPC server. The current integration suite exercises stream lifecycle through the old constructors but does not yet cover thestream_builder()path.Add compile-fail tests (via
trybuild) to verify that invalid typestate combinations produce compiler errors. Currently correctness is enforced by the type system but not explicitly tested.Typed stream returns:
build()currently returnsZerobusStreamfor both JSON and Proto, so sending the wrong payload format is still a runtime error. A future version could returnZerobusJsonStream/ZerobusProtoStream(similar to what Java already has) to make format mismatches a compile error. Candidate for 2.0.Extract shared config fields (
recovery,recovery_timeout_ms,recovery_backoff_ms,recovery_retries,server_lack_of_ack_timeout_ms,flush_timeout_ms) into aSharedStreamConfigstruct that bothStreamConfigurationOptionsandArrowStreamConfigurationOptionscompose. Currently each shared setter dual-writes to both config structs.Add idiomatic stream builder APIs to wrapper SDKs (Python, Java, Go, TypeScript), each targeting the shared internal abstraction from item 1. These should not attempt to mirror the Rust typestate — each language should use its own idiomatic pattern (kwargs in Python, traditional builder in Java, functional options in Go, options object in TypeScript).
Remove
.options()escape hatch and deprecatedcreate_stream*methods in 2.0, after wrapper SDKs are migrated.