You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "dillitz (via GitHub)" <gi...@apache.org> on 2023/07/05 21:08:36 UTC
[GitHub] [spark] dillitz opened a new pull request, #41866: [SPARK-44312][CONNECT][PYTHON] Allow to set a user agent with an environment variable
dillitz opened a new pull request, #41866:
URL: https://github.com/apache/spark/pull/41866
### What changes were proposed in this pull request?
Use the `SPARK_CONNECT_USER_AGENT` environment variable as a fallback for the prepended user agent string if one did not set the user agent in `ChannelBuilder`.
### Why are the changes needed?
Currently one has to specify a custom user agent string in `ChannelBuilder`. It would be useful to be able to set this string with an environment variable.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Manual testing + existing 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] HyukjinKwon commented on pull request #41866: [SPARK-44312][CONNECT][PYTHON] Allow to set a user agent with an environment variable
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41866:
URL: https://github.com/apache/spark/pull/41866#issuecomment-1622694774
I mean fine with this but probably we should at least document this somewhere. cc @nija-at and @grundprinzip FYI
--
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 #41866: [SPARK-44312][CONNECT][PYTHON] Allow to set a user agent with an environment variable
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #41866: [SPARK-44312][CONNECT][PYTHON] Allow to set a user agent with an environment variable
URL: https://github.com/apache/spark/pull/41866
--
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 #41866: [SPARK-44312][CONNECT][PYTHON] Allow to set a user agent with an environment variable
Posted by "nija-at (via GitHub)" <gi...@apache.org>.
nija-at commented on code in PR #41866:
URL: https://github.com/apache/spark/pull/41866#discussion_r1253932386
##########
python/pyspark/sql/connect/client/core.py:
##########
@@ -296,7 +296,10 @@ def userAgent(self) -> str:
or "_SPARK_CONNECT_PYTHON" when not specified.
The returned value will be percent encoded.
"""
- user_agent = self.params.get(ChannelBuilder.PARAM_USER_AGENT, "_SPARK_CONNECT_PYTHON")
+ user_agent = self.params.get(
+ ChannelBuilder.PARAM_USER_AGENT,
+ os.getenv("SPARK_CONNECT_USER_AGENT", "_SPARK_CONNECT_PYTHON"),
Review Comment:
Would like to call this `SPARK_CONNECT_USER_AGENT_EXTRA`.
@grundprinzip - Is that acceptable to you?
##########
python/pyspark/sql/connect/client/core.py:
##########
@@ -296,7 +296,10 @@ def userAgent(self) -> str:
or "_SPARK_CONNECT_PYTHON" when not specified.
The returned value will be percent encoded.
"""
- user_agent = self.params.get(ChannelBuilder.PARAM_USER_AGENT, "_SPARK_CONNECT_PYTHON")
+ user_agent = self.params.get(
+ ChannelBuilder.PARAM_USER_AGENT,
+ os.getenv("SPARK_CONNECT_USER_AGENT", "_SPARK_CONNECT_PYTHON"),
Review Comment:
The value of the env var needs to be appended to the existing user agent, not replaced.
--
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 #41866: [SPARK-44312][CONNECT][PYTHON] Allow to set a user agent with an environment variable
Posted by "nija-at (via GitHub)" <gi...@apache.org>.
nija-at commented on PR #41866:
URL: https://github.com/apache/spark/pull/41866#issuecomment-1622975557
@dillitz - let's chat offline about this change. It's missing some context.
--
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] dillitz commented on a diff in pull request #41866: [SPARK-44312][CONNECT][PYTHON] Allow to set a user agent with an environment variable
Posted by "dillitz (via GitHub)" <gi...@apache.org>.
dillitz commented on code in PR #41866:
URL: https://github.com/apache/spark/pull/41866#discussion_r1254107904
##########
python/pyspark/sql/connect/client/core.py:
##########
@@ -296,7 +296,10 @@ def userAgent(self) -> str:
or "_SPARK_CONNECT_PYTHON" when not specified.
The returned value will be percent encoded.
"""
- user_agent = self.params.get(ChannelBuilder.PARAM_USER_AGENT, "_SPARK_CONNECT_PYTHON")
+ user_agent = self.params.get(
+ ChannelBuilder.PARAM_USER_AGENT,
+ os.getenv("SPARK_CONNECT_USER_AGENT", "_SPARK_CONNECT_PYTHON"),
Review Comment:
It gets prepended [here](https://github.com/apache/spark/blob/8d046de2cf2fe428c3eff1696f899eff30d0b4b7/python/pyspark/sql/connect/client/core.py#L290) to the rest of the current `userAgent`. The environment variable only gets used as a fallback option if the user did not specify their own user_agent in the provided `ChannelBuilder`.
--
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 #41866: [SPARK-44312][CONNECT][PYTHON] Allow to set a user agent with an environment variable
Posted by "nija-at (via GitHub)" <gi...@apache.org>.
nija-at commented on code in PR #41866:
URL: https://github.com/apache/spark/pull/41866#discussion_r1253934601
##########
python/pyspark/sql/connect/client/core.py:
##########
@@ -296,7 +296,10 @@ def userAgent(self) -> str:
or "_SPARK_CONNECT_PYTHON" when not specified.
The returned value will be percent encoded.
"""
- user_agent = self.params.get(ChannelBuilder.PARAM_USER_AGENT, "_SPARK_CONNECT_PYTHON")
+ user_agent = self.params.get(
+ ChannelBuilder.PARAM_USER_AGENT,
+ os.getenv("SPARK_CONNECT_USER_AGENT", "_SPARK_CONNECT_PYTHON"),
Review Comment:
nvm. I found my answer elsewhere.
--
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 #41866: [SPARK-44312][CONNECT][PYTHON] Allow to set a user agent with an environment variable
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41866:
URL: https://github.com/apache/spark/pull/41866#issuecomment-1624578319
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