-
Notifications
You must be signed in to change notification settings - Fork 2
feat(sdk)!: switch to connect-rpc
for GRPC
#244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ad65930
to
cb472ef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the SDK to use connect-rpc for GRPC communication while also reorganizing the codebase to use generated service definitions instead of shipping test-only code. The key changes include replacing gRPC stubs with connect-rpc clients, updating buf configuration files to version v2 with new generation rules, and aligning example modules to use the new blocking API calls.
Reviewed Changes
Copilot reviewed 30 out of 35 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java | Migrates from gRPC channels to connect-rpc with updated blocking calls and error handling. |
sdk/src/main/java/io/opentdf/platform/sdk/Config.java | Updates import references and minor code cleanup. |
sdk/src/main/java/io/opentdf/platform/sdk/Autoconfigure.java | Replaces policy package imports with generated equivalents and updates service calls. |
sdk/src/main/java/io/opentdf/platform/sdk/AddressNormalizer.java | Introduces a new utility for normalizing addresses compatible with plaintext settings. |
sdk/buf.yaml and sdk/buf.gen.yaml | Upgrades configuration versions and adjusts generation options. |
examples/* | Updates example calls to use the new connect-rpc blocking API methods. |
Files not reviewed (5)
- examples/buf.gen.yaml: Language not supported
- examples/buf.yaml: Language not supported
- examples/pom.xml: Language not supported
- pom.xml: Language not supported
- sdk/pom.xml: Language not supported
Comments suppressed due to low confidence (2)
sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java:105
- [nitpick] Consider using a unified error handling utility; here 'RequestHelper.getOrThrow' is used while other parts of the codebase use 'ResponseMessageKt.getOrThrow'.
resp = RequestHelper.getOrThrow(req);
sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java:261
- [nitpick] Consider using consistent variable naming for request objects. The variable 'req' is renamed to 'request' in close proximity, which might be confusing.
var request = getStub(keyAccess.url).rewrapBlocking(req, Collections.emptyMap()).execute();
connect-rpc
for GRPCconnect-rpc
for GRPC
connect-rpc
for GRPCconnect-rpc
for GRPC
connect-rpc
for GRPCconnect-rpc
for GRPC
sdk/src/main/java/io/opentdf/platform/sdk/AddressNormalizer.java
Outdated
Show resolved
Hide resolved
sdk/src/main/java/io/opentdf/platform/sdk/AddressNormalizer.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates the GRPC client implementation to use connect‑rpc along with several dependency and configuration updates. Key changes include:
- Replacing gRPC channel and stub creation with OkHttpClient and ProtocolClient.
- Upgrading buf configuration to v2 and updating code generation settings.
- Adjusting address normalization logic and updating example usages to call new blocking APIs.
Reviewed Changes
Copilot reviewed 32 out of 37 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java | Updated KASClient constructor, stub retrieval, and close logic to use connect‑rpc. |
sdk/src/main/java/io/opentdf/platform/sdk/Config.java | Cleaned up redundant imports. |
sdk/src/main/java/io/opentdf/platform/sdk/Autoconfigure.java | Updated attribute grants retrieval to use blocking client APIs. |
sdk/src/main/java/io/opentdf/platform/sdk/AddressNormalizer.java | Added new address normalization logic using URI with updated scheme handling. |
sdk/buf.yaml and sdk/buf.gen.yaml | Upgraded buf version and reconfigured code generation plugins. |
examples/… | Updated examples to use new blocking methods and connect‑rpc ResponseMessageKt utilities. |
Files not reviewed (5)
- examples/buf.gen.yaml: Language not supported
- examples/buf.yaml: Language not supported
- examples/pom.xml: Language not supported
- pom.xml: Language not supported
- sdk/pom.xml: Language not supported
Comments suppressed due to low confidence (2)
sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java:252
- [nitpick] The use of 'req' followed by 'request' in the same method can be confusing; consider using a consistent or more descriptive naming scheme to improve clarity.
var req = RewrapRequest.newBuilder()
sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java:44
- Changing KASClient visibility from public to package-private may limit external accessibility. Confirm if this change is intentional or consider reverting to public.
class KASClient implements SDK.KAS {
Co-authored-by: Dave Mihalcik <dmihalcik@virtru.com>
|
TokenSource
out ofGRPCAuthInterceptor
. This lets us keep the token code in Java while using idiomatic Kotlin stuff to modify the requestbuf changes
buf
tov2
buf
generation fromexamples
. Unless we need to call tagging or something we already have everything we need insdk
dependency updates
okhttp3
depends on version1.9
of Kotlin.connect-rpc
depends on version2.1
but it's ok forokhttp3
to use a later version so we exclude itskotlin
dependency.connect-rpc
depends onokhttp3:4.12.0
we downgrade our dependency instead of forcing connect to do a major version bumpaddress normalization
http
vshttps
) from the value ofusePlaintext
usePlaintext
(true
->80
,false
->443
)