grundprinzip commented on code in PR #40150:
URL: https://github.com/apache/spark/pull/40150#discussion_r1116933885
##########
connector/connect/common/src/main/protobuf/spark/connect/base.proto:
##########
@@ -183,6 +183,141 @@ message ExecutePlanResponse {
}
}
+// The placeholder for the config request and response when the values can be
optional.
+message OptionalValue {
+ optional string value = 1;
+}
+
+// The key-value pair for the config request and response.
+message KeyValue {
+ string key = 1;
+ string value = 2;
+}
+
+// The key-value pair for the config request and response when the value can
be optional.
+message OptionalKeyValue {
+ string key = 1;
+ OptionalValue value = 2;
+}
+
+// Request to update or fetch the configurations.
+message ConfigRequest {
+ // (Required)
+ //
+ // The client_id is set by the client to be able to collate streaming
responses from
+ // different queries.
+ string client_id = 1;
+
+ // (Required) User context
+ UserContext user_context = 2;
+
+ // (Required) The operation for the config.
+ Operation operation = 3;
+
+ // Provides optional information about the client sending the request. This
field
+ // can be used for language or version specific information and is only
intended for
+ // logging purposes and will not be interpreted by the server.
+ optional string client_type = 4;
+
+ message Operation {
+ oneof op_type {
+ Set set = 1;
+ Get get = 2;
+ GetWithDefault get_with_default = 3;
+ GetOption get_option = 4;
+ GetAll get_all = 5;
+ Unset unset = 6;
+ IsModifiable is_modifiable = 7;
+ }
+ }
+
+ message Set {
+ // (Required) The config key-value pairs to set.
+ repeated KeyValue pairs = 1;
+ }
+
+ message Get {
+ // (Required) The config keys to get.
+ repeated string keys = 1;
+ }
+
+ message GetWithDefault {
+ // (Required) The config key-value paris to get. The value will be used as
the default value.
+ repeated OptionalKeyValue pairs = 1;
+ }
+
+ message GetOption {
+ // (Required) The config keys to get optionally.
+ repeated string keys = 1;
+ }
+
+ message GetAll {
+ // (Optional) The prefix of the config key to get.
+ optional string prefix = 1;
+ }
+
+ message Unset {
+ // (Required) The config keys to unset.
+ repeated string keys = 1;
+ }
+
+ message IsModifiable {
+ // (Required) The config keys to check the config is modifiable.
+ repeated string keys = 1;
+ }
+}
+
+// Response to the config request.
+message ConfigResponse {
+ string client_id = 1;
+
+ // (Required) The operation for the config.
+ Operation operation = 2;
+
+ // (Optional)
+ //
+ // Warning messages for deprecated or unsupported configurations.
+ repeated string warnings = 3;
+
+ message Operation {
+ oneof op_type {
+ Set set = 1;
+ Get get = 2;
+ GetWithDefault get_with_default = 3;
+ GetOption get_option = 4;
+ GetAll get_all = 5;
+ Unset unset = 6;
+ IsModifiable is_modifiable = 7;
+ }
+ }
Review Comment:
Shouldn't the ConfigResponse simply be
```
repeated KeyValue values
```
?
##########
connector/connect/common/src/main/protobuf/spark/connect/base.proto:
##########
@@ -183,6 +183,141 @@ message ExecutePlanResponse {
}
}
+// The placeholder for the config request and response when the values can be
optional.
+message OptionalValue {
+ optional string value = 1;
+}
+
+// The key-value pair for the config request and response.
+message KeyValue {
+ string key = 1;
+ string value = 2;
+}
+
+// The key-value pair for the config request and response when the value can
be optional.
+message OptionalKeyValue {
+ string key = 1;
+ OptionalValue value = 2;
+}
+
Review Comment:
My suggestion would be to simply make the value optional in the KeyValue
pair. Since proto does not have required fields per se, this avoids some of the
repetion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
