You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "WweiL (via GitHub)" <gi...@apache.org> on 2024/02/13 20:09:16 UTC

[PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

WweiL opened a new pull request, #45091:
URL: https://github.com/apache/spark/pull/45091

   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
     8. If you want to add or modify an error type or message, please read the guideline first in
        'common/utils/src/main/resources/error/README.md'.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   Currently, the StreamingQueryListener for Connect runs on the server side.
   
   From a customer point of view, the purpose of a listener is mainly to have a pushing mechanism that notifies them when queries start / end / make progress. Before, the server-side listener essentially loses this functionality, and we find out that internally there are needs for the client side listener.
   
   The new listener will be running on the client side, with the server continuously pushing the new listener events to the client, and the client will call corresponding callback functions for different listener event types.
   
   This is the first PR that defines the protocol of this new listener.
   
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   Add client side listener which makes more sense.
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   Not for this one.
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
   -->
   No need for this one, new tests will be added later.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   <!--
   If generative AI tooling has been used in the process of authoring this patch, please include the
   phrase: 'Generated-by: ' followed by the name of the tool and its version.
   If no, write 'No'.
   Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
   -->
   No


-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "WweiL (via GitHub)" <gi...@apache.org>.
WweiL commented on PR #45091:
URL: https://github.com/apache/spark/pull/45091#issuecomment-1942363588

   @grundprinzip 


-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "WweiL (via GitHub)" <gi...@apache.org>.
WweiL commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1498959290


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,32 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The protocol for the returned events in the long-running response channel.
+// The event_type for QueryProgressEvent is 1;
+// for QueryTerminatedEvent is 2; for QueryIdleEvent is 3;
+// There is no QueryStartedEvent defined here,

Review Comment:
   Yes we can use similar mechanism as PythonEvalType
   https://github.com/databricks/runtime/blob/95f1afae584a89d20ecf2e411f8b3a267d354e46/python/pyspark/rdd.py#L141



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin closed pull request #45091: [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener
URL: https://github.com/apache/spark/pull/45091


-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1501037087


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,39 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The enum used for client side streaming query listener event
+// There is no QueryStartedEvent defined here,
+// it is added as a field in WriteStreamOperationStartResult
+enum StreamingQueryEventType {
+  QUERY_PROGRESS_EVENT = 0;
+  QUERY_TERMINATED_EVENT = 1;
+  QUERY_IDLE_EVENT = 2;
+}

Review Comment:
   Also, each variable should have the same prefix as the enum name? I'm not sure if this is a strict rule or not, though. @amaliujia 



##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,39 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The enum used for client side streaming query listener event
+// There is no QueryStartedEvent defined here,
+// it is added as a field in WriteStreamOperationStartResult
+enum StreamingQueryEventType {
+  QUERY_PROGRESS_EVENT = 0;
+  QUERY_TERMINATED_EVENT = 1;
+  QUERY_IDLE_EVENT = 2;
+}

Review Comment:
   We need something like `QUERY_EVENT_UNSPECIFIED` as `0`?
   https://protobuf.dev/programming-guides/proto3/#enum



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1501037087


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,39 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The enum used for client side streaming query listener event
+// There is no QueryStartedEvent defined here,
+// it is added as a field in WriteStreamOperationStartResult
+enum StreamingQueryEventType {
+  QUERY_PROGRESS_EVENT = 0;
+  QUERY_TERMINATED_EVENT = 1;
+  QUERY_IDLE_EVENT = 2;
+}

Review Comment:
   Also, each variable should have the same prefix as the enum name? I'm not sure if this is a strict rule or not, though. @grundprinzip @amaliujia 



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1501051706


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,39 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The enum used for client side streaming query listener event
+// There is no QueryStartedEvent defined here,
+// it is added as a field in WriteStreamOperationStartResult
+enum StreamingQueryEventType {
+  QUERY_PROGRESS_EVENT = 0;
+  QUERY_TERMINATED_EVENT = 1;
+  QUERY_IDLE_EVENT = 2;
+}

Review Comment:
   +1 for `QUERY_EVENT_UNSPECIFIED = 0`.
   
   No IIRC we didn't have a strict rule to ask each variable have the same prefix as the enum name.



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1498950023


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,32 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The protocol for the returned events in the long-running response channel.
+// The event_type for QueryProgressEvent is 1;
+// for QueryTerminatedEvent is 2; for QueryIdleEvent is 3;
+// There is no QueryStartedEvent defined here,
+// it is added as a field in WriteStreamOperationStartResult
+message StreamingQueryListenerEvents {
+  string event_json = 1;

Review Comment:
   Is this guaranteed to be JSON?



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "WweiL (via GitHub)" <gi...@apache.org>.
WweiL commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1498956792


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,32 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The protocol for the returned events in the long-running response channel.
+// The event_type for QueryProgressEvent is 1;
+// for QueryTerminatedEvent is 2; for QueryIdleEvent is 3;
+// There is no QueryStartedEvent defined here,
+// it is added as a field in WriteStreamOperationStartResult
+message StreamingQueryListenerEvents {
+  string event_json = 1;

Review Comment:
   Yes all streaming query events have a json method https://github.com/apache/spark/blob/fd518dac8d9f9a117e06319e768c2c80f8ce600f/sql/core/src/main/scala/org/apache/spark/sql/streaming/StreamingQueryListener.scala#L127
   
   on client there are fromJson methods to reconstruct the event:
   https://github.com/apache/spark/blob/bbf06c867e0e2f68d3b3b86080e54a16e6213235/python/pyspark/sql/streaming/listener.py#L194



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on PR #45091:
URL: https://github.com/apache/spark/pull/45091#issuecomment-1961872635

   LGTM once CI pass, thank you!


-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "WweiL (via GitHub)" <gi...@apache.org>.
WweiL commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1499829242


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,32 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The protocol for the returned events in the long-running response channel.
+// The event_type for QueryProgressEvent is 1;
+// for QueryTerminatedEvent is 2; for QueryIdleEvent is 3;
+// There is no QueryStartedEvent defined here,

Review Comment:
   oh I see what you meant, createrd a enum in proto file



##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,32 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The protocol for the returned events in the long-running response channel.
+// The event_type for QueryProgressEvent is 1;
+// for QueryTerminatedEvent is 2; for QueryIdleEvent is 3;
+// There is no QueryStartedEvent defined here,

Review Comment:
   oh I see what you meant, created a enum in proto file



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1498948905


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,32 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The protocol for the returned events in the long-running response channel.
+// The event_type for QueryProgressEvent is 1;
+// for QueryTerminatedEvent is 2; for QueryIdleEvent is 3;
+// There is no QueryStartedEvent defined here,
+// it is added as a field in WriteStreamOperationStartResult
+message StreamingQueryListenerEvents {

Review Comment:
   ```suggestion
   message StreamingQueryListenerEvent {
   ```



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1498949593


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,32 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The protocol for the returned events in the long-running response channel.
+// The event_type for QueryProgressEvent is 1;
+// for QueryTerminatedEvent is 2; for QueryIdleEvent is 3;
+// There is no QueryStartedEvent defined here,

Review Comment:
   Should we formalize this somehow?



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "WweiL (via GitHub)" <gi...@apache.org>.
WweiL commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1498959891


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,32 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The protocol for the returned events in the long-running response channel.
+// The event_type for QueryProgressEvent is 1;
+// for QueryTerminatedEvent is 2; for QueryIdleEvent is 3;
+// There is no QueryStartedEvent defined here,

Review Comment:
   that would be a bit out of the scope of this protocol pr? 



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "WweiL (via GitHub)" <gi...@apache.org>.
WweiL commented on code in PR #45091:
URL: https://github.com/apache/spark/pull/45091#discussion_r1501064090


##########
connector/connect/common/src/main/protobuf/spark/connect/commands.proto:
##########
@@ -421,6 +424,39 @@ message StreamingQueryManagerCommandResult {
   }
 }
 
+// The protocol for client-side StreamingQueryListener.
+// This command will only be set when either the first listener is added to the client, or the last
+// listener is removed from the client.
+// The add_listener_bus_listener command will only be set true in the first case.
+// The remove_listener_bus_listener command will only be set true in the second case.
+message StreamingQueryListenerBusCommand {
+  oneof command {
+    bool add_listener_bus_listener = 1;
+    bool remove_listener_bus_listener = 2;
+  }
+}
+
+// The enum used for client side streaming query listener event
+// There is no QueryStartedEvent defined here,
+// it is added as a field in WriteStreamOperationStartResult
+enum StreamingQueryEventType {
+  QUERY_PROGRESS_EVENT = 0;
+  QUERY_TERMINATED_EVENT = 1;
+  QUERY_IDLE_EVENT = 2;
+}

Review Comment:
   Thanks for the suggestion! let me update



-- 
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


Re: [PR] [SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener [spark]

Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on PR #45091:
URL: https://github.com/apache/spark/pull/45091#issuecomment-1961985939

   Thanks! merging to master.


-- 
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