indigophox commented on code in PR #34817:
URL: https://github.com/apache/arrow/pull/34817#discussion_r1410027807


##########
format/Flight.proto:
##########
@@ -525,3 +525,108 @@ message FlightData {
 message PutResult {
   bytes app_metadata = 1;
 }
+
+/*
+ * EXPERIMENTAL: Union of possible value types for a Session Option to be set 
to.
+ */
+message SessionOptionValue {
+  message StringListValue {
+    repeated string values = 1;
+  }
+
+  oneof option_value {
+    string string_value = 1;
+    bool bool_value = 2;
+    sfixed32 int32_value = 3;
+    sfixed64 int64_value = 4;
+    float float_value = 5;
+    double double_value = 6;
+    StringListValue string_list_value = 7;
+  }
+}
+
+/*
+ * EXPERIMENTAL: A request to set session options for an existing or new 
(implicit)
+ * server session.
+ *
+ * Sessions are persisted and referenced via RFC 6265 HTTP cookies, canonically
+ * 'arrow_flight_session_id', although implementations may freely choose their 
own name.
+ */
+message SetSessionOptionsRequest {
+  map<string, SessionOptionValue> session_options = 1;
+}
+
+/*
+ * EXPERIMENTAL: The results (individually) of setting a set of session 
options.
+ */
+message SetSessionOptionsResult {
+  enum Status {
+    // The status of setting the option is unknown. Servers should avoid using
+    // this value (send a NOT_FOUND error if the requested query is
+    // not known). Clients can retry the request.
+    SET_SESSION_OPTION_RESULT_UNSPECIFIED = 0;
+    // The session option setting completed successfully.
+    SET_SESSION_OPTION_RESULT_OK = 1;
+    // The given session option name was an alias for another option name.
+    SET_SESSION_OPTION_RESULT_OK_MAPPED = 2;
+    // The given session option name is invalid.
+    SET_SESSION_OPTION_RESULT_INVALID_NAME = 3;
+    // The session option value is invalid.
+    SET_SESSION_OPTION_RESULT_INVALID_VALUE = 4;
+    // The session option cannot be set.
+    SET_SESSION_OPTION_RESULT_ERROR = 5;
+  }
+
+  message Result {
+    Status status = 1;
+  }
+  
+  map<string, Result> results = 1;
+}
+
+/*
+ * EXPERIMENTAL: A request to access the session options for the current 
server session.
+ *
+ * The existing session is referenced via a cookie header; it is an error to 
make this
+ * request with a missing, invalid, or expired session cookie header.
+ */
+message GetSessionOptionsRequest {
+}
+
+/*
+ * EXPERIMENTAL: The result containing the current server session options.
+ */
+message GetSessionOptionsResult {
+    map<string, SessionOptionValue> session_options = 1;
+}
+
+/*
+ * Request message for the "Close Session" action.
+ *
+ * The exiting session is referenced via a cookie header.
+ */
+message CloseSessionRequest {
+}
+
+/*
+ * The result of closing a session.
+ */
+message CloseSessionResult {
+  enum Status {
+    // The session close status is unknown. Servers should avoid using
+    // this value (send a NOT_FOUND error if the requested session is
+    // not known). Clients can retry the request.
+    CLOSE_RESULT_UNSPECIFIED = 0;
+    // The session close request is complete. Subsequent requests with
+    // the same session produce a NOT_FOUND error.
+    CLOSE_RESULT_CLOSED = 1;
+    // The session close request is in progress. The client may retry
+    // the close request.
+    CLOSE_RESULT_CLOSING = 2;
+    // The session is not closeable. The client should not retry the

Review Comment:
   For consistency e.g. 
https://github.com/apache/arrow/blob/d66780d90c6faf7bf051cbf12b4b68dff098bd54/format/Flight.proto#L241-L255
   
   While it's a valid point to make the documentation of these values more 
clear, I believe it needs to be done globally to avoid implying differences in 
the different enum values' meaning if we document them differently.
   
   Presumably "not yet closed" may fit e.g. @kou's use case of rolling auth 
into sessions, where closing a session would also nullify AuthNZ context 
attached to the session—it may be desirable to confirm that temporary auth 
tokens (session tokens) have in fact been successfully invalidated e.g. in a 
distributed environment.  "Cannot be closed" would then fit a situation where 
the session was previously CLOSING but acknowledgement from all nodes had not 
been received before a timeout was reached...



##########
format/Flight.proto:
##########
@@ -525,3 +525,108 @@ message FlightData {
 message PutResult {
   bytes app_metadata = 1;
 }
+
+/*
+ * EXPERIMENTAL: Union of possible value types for a Session Option to be set 
to.
+ */
+message SessionOptionValue {
+  message StringListValue {
+    repeated string values = 1;
+  }
+
+  oneof option_value {
+    string string_value = 1;
+    bool bool_value = 2;
+    sfixed32 int32_value = 3;
+    sfixed64 int64_value = 4;
+    float float_value = 5;
+    double double_value = 6;
+    StringListValue string_list_value = 7;
+  }
+}
+
+/*
+ * EXPERIMENTAL: A request to set session options for an existing or new 
(implicit)
+ * server session.
+ *
+ * Sessions are persisted and referenced via RFC 6265 HTTP cookies, canonically
+ * 'arrow_flight_session_id', although implementations may freely choose their 
own name.
+ */
+message SetSessionOptionsRequest {
+  map<string, SessionOptionValue> session_options = 1;
+}
+
+/*
+ * EXPERIMENTAL: The results (individually) of setting a set of session 
options.
+ */
+message SetSessionOptionsResult {
+  enum Status {
+    // The status of setting the option is unknown. Servers should avoid using
+    // this value (send a NOT_FOUND error if the requested query is
+    // not known). Clients can retry the request.
+    SET_SESSION_OPTION_RESULT_UNSPECIFIED = 0;
+    // The session option setting completed successfully.
+    SET_SESSION_OPTION_RESULT_OK = 1;
+    // The given session option name was an alias for another option name.
+    SET_SESSION_OPTION_RESULT_OK_MAPPED = 2;
+    // The given session option name is invalid.
+    SET_SESSION_OPTION_RESULT_INVALID_NAME = 3;
+    // The session option value is invalid.
+    SET_SESSION_OPTION_RESULT_INVALID_VALUE = 4;
+    // The session option cannot be set.
+    SET_SESSION_OPTION_RESULT_ERROR = 5;
+  }
+
+  message Result {
+    Status status = 1;
+  }
+  
+  map<string, Result> results = 1;
+}
+
+/*
+ * EXPERIMENTAL: A request to access the session options for the current 
server session.
+ *
+ * The existing session is referenced via a cookie header; it is an error to 
make this
+ * request with a missing, invalid, or expired session cookie header.
+ */
+message GetSessionOptionsRequest {
+}
+
+/*
+ * EXPERIMENTAL: The result containing the current server session options.
+ */
+message GetSessionOptionsResult {
+    map<string, SessionOptionValue> session_options = 1;
+}
+
+/*
+ * Request message for the "Close Session" action.
+ *
+ * The exiting session is referenced via a cookie header.
+ */
+message CloseSessionRequest {
+}
+
+/*
+ * The result of closing a session.
+ */
+message CloseSessionResult {
+  enum Status {
+    // The session close status is unknown. Servers should avoid using
+    // this value (send a NOT_FOUND error if the requested session is
+    // not known). Clients can retry the request.
+    CLOSE_RESULT_UNSPECIFIED = 0;
+    // The session close request is complete. Subsequent requests with
+    // the same session produce a NOT_FOUND error.
+    CLOSE_RESULT_CLOSED = 1;
+    // The session close request is in progress. The client may retry
+    // the close request.
+    CLOSE_RESULT_CLOSING = 2;
+    // The session is not closeable. The client should not retry the

Review Comment:
   For consistency e.g. 
https://github.com/apache/arrow/blob/d66780d90c6faf7bf051cbf12b4b68dff098bd54/format/Flight.proto#L241-L255
   
   While it's a valid point to make the documentation of these values more 
clear, I believe it needs to be done globally to avoid implying differences in 
the different enum values' meaning if we document them differently.
   
   Presumably "not yet closed" may fit e.g. @kou's use case of rolling auth 
into sessions, where closing a session would also nullify AuthNZ context 
attached to the session—it may be desirable to confirm that temporary auth 
tokens (session tokens) have in fact been successfully invalidated e.g. in a 
distributed environment.  "Cannot be closed" would then fit a situation where 
the session was previously CLOSING but acknowledgement from all nodes had not 
been received before a timeout was reached...



-- 
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: github-unsubscr...@arrow.apache.org

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

Reply via email to