-
Notifications
You must be signed in to change notification settings - Fork 57
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
Refactor Protobuf definitions for more idiomatic SDKs #843
Labels
Comments
This was referenced Oct 18, 2023
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 30, 2023
## Description: refactor protobuf definitions for more idiomatic SDKs ## Is this change user-facing? NO ## References (if applicable): Fix #843
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 30, 2023
🤖 I have created a release *beep* *boop* --- ## [0.85.0](0.84.13...0.85.0) (2023-10-30) ### ⚠ BREAKING CHANGES * protobuf definitions for more idiomatic SDKs ([#1586](#1586)) ### Features * Add cli argument to control image download ([#1495](#1495)) ([f210a76](f210a76)) ### Bug Fixes * run_sh doesn't remove new lines from input ([#1642](#1642)) ([a969dff](a969dff)) ### Code Refactoring * protobuf definitions for more idiomatic SDKs ([#1586](#1586)) ([e7ab58a](e7ab58a)), closes [#843](#843) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: kurtosisbot <[email protected]> Co-authored-by: leoporoli <[email protected]>
h4ck3rk3y
pushed a commit
that referenced
this issue
Oct 31, 2023
## Description: refactor protobuf definitions for more idiomatic SDKs ## Is this change user-facing? NO ## References (if applicable): Fix #843
h4ck3rk3y
pushed a commit
that referenced
this issue
Oct 31, 2023
🤖 I have created a release *beep* *boop* --- ## [0.85.0](0.84.13...0.85.0) (2023-10-30) ### ⚠ BREAKING CHANGES * protobuf definitions for more idiomatic SDKs ([#1586](#1586)) ### Features * Add cli argument to control image download ([#1495](#1495)) ([f210a76](f210a76)) ### Bug Fixes * run_sh doesn't remove new lines from input ([#1642](#1642)) ([a969dff](a969dff)) ### Code Refactoring * protobuf definitions for more idiomatic SDKs ([#1586](#1586)) ([e7ab58a](e7ab58a)), closes [#843](#843) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: kurtosisbot <[email protected]> Co-authored-by: leoporoli <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Background & motivation
For example,
CreateEnclave
call takes in a optional parameters likeapi_container_version_tag
. This field is not tagged as optional in the protobuf.This is not a problem for Golang, where you can construct a struct by passing only a subset of the parameters, defaulting omitted values. In this specific case, building
CreateEnclaveRequest
omittingapi_container_version_tag
defaults it to""
, which we interpret asempty
in the backend.In Rust, this is problematic, since there is a clear difference between null value (
""
) and absent value (Option::None
), as seen in our current SDK: https://crates.io/crates/kurtosis-sdk/0.80.7Desired behaviour
Review our APIs, adding explicit
optional
field for elements that can be semantically absent (optional) vs protocol absent (regular field). See https://protobuf.dev/programming-guides/proto3/#field-labelsHow important is this to you?
Nice to have; this feature would make using Kurtosis more enjoyable.
The text was updated successfully, but these errors were encountered: