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/05/11 10:32:12 UTC
[GitHub] [spark] nija-at opened a new pull request, #41138: [PYTHON][SPARK-43457][CONNECT] Augument user agent with OS and Python versions
nija-at opened a new pull request, #41138:
URL: https://github.com/apache/spark/pull/41138
### What changes were proposed in this pull request?
Augument the user agent string sent over the service to include
operating system and Python version.
### Why are the changes needed?
Including OS and Python versions in the user agent improves
tracking to see how Spark Connect is used across Python versions
and platforms.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit tests attached.
--
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 #41138: [SPARK-43457][CONNECT][PYTHON] Augument user agent with OS, Python and Spark versions
Posted by "nija-at (via GitHub)" <gi...@apache.org>.
nija-at commented on code in PR #41138:
URL: https://github.com/apache/spark/pull/41138#discussion_r1193456123
##########
python/pyspark/sql/connect/client.py:
##########
@@ -299,7 +301,12 @@ def userAgent(self) -> str:
raise SparkConnectException(
f"'user_agent' parameter should not exceed 2048 characters, found {len} characters."
)
- return user_agent
+ return " ".join([
Review Comment:
This is the IETF standard format for 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] nija-at commented on a diff in pull request #41138: [SPARK-43457][CONNECT][PYTHON] Augument user agent with OS and Python versions
Posted by "nija-at (via GitHub)" <gi...@apache.org>.
nija-at commented on code in PR #41138:
URL: https://github.com/apache/spark/pull/41138#discussion_r1192190526
##########
python/pyspark/sql/connect/client.py:
##########
@@ -299,7 +300,11 @@ def userAgent(self) -> str:
raise SparkConnectException(
f"'user_agent' parameter should not exceed 2048 characters, found {len} characters."
)
- return user_agent
+ return " ".join([
+ user_agent,
Review Comment:
Don't think it's wise to depend on the order of fields.
--
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 #41138: [SPARK-43457][CONNECT][PYTHON] Augument user agent with OS, Python and Spark versions
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41138:
URL: https://github.com/apache/spark/pull/41138#issuecomment-1549237396
Merged 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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #41138: [SPARK-43457][CONNECT][PYTHON] Augument user agent with OS, Python and Spark versions
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #41138:
URL: https://github.com/apache/spark/pull/41138#discussion_r1193232765
##########
python/pyspark/sql/connect/client.py:
##########
@@ -299,7 +301,12 @@ def userAgent(self) -> str:
raise SparkConnectException(
f"'user_agent' parameter should not exceed 2048 characters, found {len} characters."
)
- return user_agent
+ return " ".join([
Review Comment:
Don't have a strong opinion but wonder if we should return a dictionary or at least JSON string in case it's programmatically used
--
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 #41138: [PYTHON][SPARK-43457][CONNECT] Augument user agent with OS and Python versions
Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #41138:
URL: https://github.com/apache/spark/pull/41138#discussion_r1191346077
##########
python/pyspark/sql/connect/client.py:
##########
@@ -299,7 +300,11 @@ def userAgent(self) -> str:
raise SparkConnectException(
f"'user_agent' parameter should not exceed 2048 characters, found {len} characters."
)
- return user_agent
+ return " ".join([
+ user_agent,
Review Comment:
if we put the other two values first then we have "guarantee" on the os and python version as the first two fields.
In addition, we should put the spark version here as well -> `from pyspark.version import __version__`
--
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 #41138: [SPARK-43457][CONNECT][PYTHON] Augument user agent with OS, Python and Spark versions
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #41138: [SPARK-43457][CONNECT][PYTHON] Augument user agent with OS, Python and Spark versions
URL: https://github.com/apache/spark/pull/41138
--
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 pull request #41138: [PYTHON][SPARK-43457][CONNECT] Augument user agent with OS and Python versions
Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on PR #41138:
URL: https://github.com/apache/spark/pull/41138#issuecomment-1544188704
Please change the subject: `[SPARK-43457][CONNECT][PYTHON]`
--
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 #41138: [SPARK-43457][CONNECT][PYTHON] Augument user agent with OS, Python and Spark versions
Posted by "nija-at (via GitHub)" <gi...@apache.org>.
nija-at commented on code in PR #41138:
URL: https://github.com/apache/spark/pull/41138#discussion_r1192194721
##########
python/pyspark/sql/connect/client.py:
##########
@@ -299,7 +300,11 @@ def userAgent(self) -> str:
raise SparkConnectException(
f"'user_agent' parameter should not exceed 2048 characters, found {len} characters."
)
- return user_agent
+ return " ".join([
+ user_agent,
Review Comment:
> In addition, we should put the spark version here as well
Done
--
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