You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "nija-at (via GitHub)" <gi...@apache.org> on 2023/02/16 14:21:19 UTC
[GitHub] [spark] nija-at opened a new pull request, #40054: pyspark: accept user_agent in spark connect's connection string
nija-at opened a new pull request, #40054:
URL: https://github.com/apache/spark/pull/40054
### What changes were proposed in this pull request?
Currently, the Spark Connect service's `client_type` attribute (which is really [user
agent]) is set to `_SPARK_CONNECT_PYTHON` to signify PySpark.
With this change, the connection for the Spark Connect remote accepts an optional
`user_agent` parameter which is then passed down to the service.
[user agent]: https://www.w3.org/WAI/UA/work/wiki/Definition_of_User_Agent
### Why are the changes needed?
This enables partners using Spark Connect to set their application as the user agent,
which then allows visibility and measurement of integrations and usages of spark
connect.
### Does this PR introduce _any_ user-facing change?
A new optional `user_agent` parameter is now recognized as part of the Spark Connect
connection string.
### How was this patch tested?
- unit tests attached
- manually running the `pyspark` binary with the `user_agent` connection string set and
verifying the payload sent to the server. Similar testing for the default.
--
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
[GitHub] [spark] nija-at commented on a diff in pull request #40054: pyspark: accept user_agent in spark connect's connection string
Posted by "nija-at (via GitHub)" <gi...@apache.org>.
nija-at commented on code in PR #40054:
URL: https://github.com/apache/spark/pull/40054#discussion_r1108553346
##########
python/pyspark/sql/connect/client.py:
##########
@@ -244,6 +246,17 @@ def userId(self) -> Optional[str]:
"""
return self.params.get(ChannelBuilder.PARAM_USER_ID, None)
+ @property
+ def userAgent(self) -> str:
+ """
+ Returns
+ -------
+ user_agent : str
+ The user_agent parameter specified in the connection string,
+ or "_SPARK_CONNECT_PYTHON" when not specified.
+ """
+ return self.params.get(ChannelBuilder.PARAM_USER_AGENT, "_SPARK_CONNECT_PYTHON")
Review Comment:
TODO: apply some reasonable validation on the length and charset of user agent
--
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
[GitHub] [spark] amaliujia commented on pull request #40054: pyspark: accept user_agent in spark connect's connection string
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #40054:
URL: https://github.com/apache/spark/pull/40054#issuecomment-1433779252
Actually. Please open a JIRA and add JIRA id to the PR title along with module names (example: `[SPARK-xxxx][CONNECT][PYTHON]) accept user_agent in spark connect's connection string`)
--
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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #40054: pyspark: accept user_agent in spark connect's connection string
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #40054:
URL: https://github.com/apache/spark/pull/40054#discussion_r1109190403
##########
python/pyspark/sql/tests/connect/test_client.py:
##########
@@ -0,0 +1,50 @@
+import unittest
+
+from pyspark.sql.connect.client import SparkConnectClient
+import pyspark.sql.connect.proto as proto
+
+
+class SparkConnectClientTestCase(unittest.TestCase):
+ def test_user_agent_passthrough(self):
+ client = SparkConnectClient('sc://foo/;user_agent=bar')
+ mock = MockService(client._session_id)
+ client._stub = mock
+
+ command = proto.Command()
+ client.execute_command(command)
+
+ self.assertIsNotNone(mock.req, "ExecutePlan API was not called when expected")
+ self.assertEqual(mock.req.client_type, "bar")
+
+ def test_user_agent_default(self):
+ client = SparkConnectClient('sc://foo/')
+ mock = MockService(client._session_id)
+ client._stub = mock
+
+ command = proto.Command()
+ client.execute_command(command)
+
+ self.assertIsNotNone(mock.req, "ExecutePlan API was not called when expected")
+ self.assertEqual(mock.req.client_type, "_SPARK_CONNECT_PYTHON")
+
+
+class MockService:
+ """
+ Simplest mock of the SparkConnectService.
+ If this needs more complex logic, it needs to be replaced with Python mocking.
+ """
+ req: proto.ExecutePlanRequest | None
Review Comment:
we don't need type hints in tests
--
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
[GitHub] [spark] amaliujia commented on a diff in pull request #40054: pyspark: accept user_agent in spark connect's connection string
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #40054:
URL: https://github.com/apache/spark/pull/40054#discussion_r1108954946
##########
connector/connect/docs/client-connection-string.md:
##########
@@ -81,6 +82,15 @@ sc://hostname:port/;param1=value;param2=value
<pre>user_id=Martin</pre>
</td>
</tr>
+ <tr>
+ <td>user_agent</td>
+ <td>String</td>
+ <td>The user agent acting on behalf of the user, typically applications
+ that use Spark Connect to implement its functionality and execute Spark
+ requests on behalf of the user.<br/>
+ <i>Default: </i>_SPARK_CONNECT_PYTHON</td>
Review Comment:
+1
--
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
[GitHub] [spark] nija-at commented on pull request #40054: [SPARK-42477] [CONNECT] [PYTHON]: accept user_agent in spark connect's connection string
Posted by "nija-at (via GitHub)" <gi...@apache.org>.
nija-at commented on PR #40054:
URL: https://github.com/apache/spark/pull/40054#issuecomment-1434552098
@hvanhovell
> can you file a ticket in the JIRA (or ask me to do it), and add that to the title?
Done.
> Can you also do this for the scala client?
I'll do that as a separate change
--
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
[GitHub] [spark] hvanhovell commented on pull request #40054: pyspark: accept user_agent in spark connect's connection string
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #40054:
URL: https://github.com/apache/spark/pull/40054#issuecomment-1433329522
@nija-at can you file a ticket in the JIRA (or ask me to do it), and add that to the title?
--
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
[GitHub] [spark] amaliujia commented on a diff in pull request #40054: pyspark: accept user_agent in spark connect's connection string
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #40054:
URL: https://github.com/apache/spark/pull/40054#discussion_r1109069489
##########
python/pyspark/sql/tests/connect/test_client.py:
##########
@@ -0,0 +1,50 @@
+import unittest
Review Comment:
You need add Apache license at the top.
--
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
[GitHub] [spark] grundprinzip commented on a diff in pull request #40054: pyspark: accept user_agent in spark connect's connection string
Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #40054:
URL: https://github.com/apache/spark/pull/40054#discussion_r1108868663
##########
connector/connect/docs/client-connection-string.md:
##########
@@ -81,6 +82,15 @@ sc://hostname:port/;param1=value;param2=value
<pre>user_id=Martin</pre>
</td>
</tr>
+ <tr>
+ <td>user_agent</td>
+ <td>String</td>
+ <td>The user agent acting on behalf of the user, typically applications
+ that use Spark Connect to implement its functionality and execute Spark
+ requests on behalf of the user.<br/>
+ <i>Default: </i>_SPARK_CONNECT_PYTHON</td>
Review Comment:
It's only the default in the Python client. :) The proto itself is probably just empty.
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #40054: [SPARK-42477][CONNECT][PYTHON] accept user_agent in spark connect's connection string
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #40054:
URL: https://github.com/apache/spark/pull/40054#issuecomment-1437070103
Merged to master and branch-3.4.
--
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
[GitHub] [spark] amaliujia commented on a diff in pull request #40054: [SPARK-42477] [CONNECT] [PYTHON]: accept user_agent in spark connect's connection string
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #40054:
URL: https://github.com/apache/spark/pull/40054#discussion_r1110284777
##########
connector/connect/docs/client-connection-string.md:
##########
@@ -81,6 +82,15 @@ sc://hostname:port/;param1=value;param2=value
<pre>user_id=Martin</pre>
</td>
</tr>
+ <tr>
+ <td>user_agent</td>
+ <td>String</td>
+ <td>The user agent acting on behalf of the user, typically applications
+ that use Spark Connect to implement its functionality and execute Spark
+ requests on behalf of the user.<br/>
+ <i>Default: </i><pre>_SPARK_CONNECT_PYTHON</pre> in the Python client</td>
Review Comment:
Wait I thought this is addressed: the default value is empty right? `_SPARK_CONNECT_PYTHON` is Python client set value and Scala client for example will set a different value.
--
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
[GitHub] [spark] amaliujia commented on a diff in pull request #40054: [SPARK-42477] [CONNECT] [PYTHON]: accept user_agent in spark connect's connection string
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #40054:
URL: https://github.com/apache/spark/pull/40054#discussion_r1110286421
##########
connector/connect/docs/client-connection-string.md:
##########
@@ -81,6 +82,15 @@ sc://hostname:port/;param1=value;param2=value
<pre>user_id=Martin</pre>
</td>
</tr>
+ <tr>
+ <td>user_agent</td>
+ <td>String</td>
+ <td>The user agent acting on behalf of the user, typically applications
+ that use Spark Connect to implement its functionality and execute Spark
+ requests on behalf of the user.<br/>
+ <i>Default: </i><pre>_SPARK_CONNECT_PYTHON</pre> in the Python client</td>
Review Comment:
I see that this is changed to default value in Python client.
This is fine. Probably in the future we just list the default value for the Scala client.
--
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
[GitHub] [spark] hvanhovell commented on pull request #40054: pyspark: accept user_agent in spark connect's connection string
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #40054:
URL: https://github.com/apache/spark/pull/40054#issuecomment-1433330261
Can you also do this for the scala client?
--
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
[GitHub] [spark] HyukjinKwon closed pull request #40054: [SPARK-42477][CONNECT][PYTHON] accept user_agent in spark connect's connection string
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #40054: [SPARK-42477][CONNECT][PYTHON] accept user_agent in spark connect's connection string
URL: https://github.com/apache/spark/pull/40054
--
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