Re: [PR] [SPARK-53796] Add `extension` field to pipeline protos to support forward compatibility [spark]

["sryza (via GitHub)" <gi...@apache.org>]

sryza commented on code in PR #52514:
URL: https://github.com/apache/spark/pull/52514#discussion_r2403001487


##########
sql/connect/common/src/main/protobuf/spark/connect/pipelines.proto:
##########
@@ -41,6 +42,12 @@ message PipelineCommand {
     DefineFlowQueryFunctionResult define_flow_query_function_result = 8;
   }
 
+  // Reserved field for protocol extensions.
+  // Used to support forward-compatibility by carrying additional command types
+  // that are not yet defined in this version of the proto. During planning, the
+  // engine will resolve and dispatch the concrete command contained in this field.
+  google.protobuf.Any extension = 999;

Review Comment:
   Should this be part of the `command_type` `oneof`? Seems like this would be mutually exclusive with the existing set of commands?



##########
sql/connect/common/src/main/protobuf/spark/connect/pipelines.proto:
##########
@@ -89,6 +96,12 @@ message PipelineCommand {
 
     // The location in source code that this dataset was defined.
     optional SourceCodeLocation source_code_location = 9;
+
+    // Reserved field for protocol extensions.
+    // Used to support forward-compatibility by carrying additional fields
+    // that are not yet defined in this version of the proto. During planning, the
+    // engine will resolve and dispatch the concrete command contained in this field.
+    google.protobuf.Any extension = 999;

Review Comment:
   It looks like best practice is to use `repeated` for extensions that aren't part of a `oneof`: https://github.com/apache/spark/blob/master/sql/connect/common/src/main/protobuf/spark/connect/base.proto#L58



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org