You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/08/13 23:04:51 UTC
[GitHub] [spark] dnskr opened a new pull request #33740: [SPARK-36510][DOCS] Add spark.redaction.string.regex property to the docs
dnskr opened a new pull request #33740:
URL: https://github.com/apache/spark/pull/33740
### What changes were proposed in this pull request?
The PR fixes [SPARK-36510](https://issues.apache.org/jira/browse/SPARK-36510) by adding missing `spark.redaction.string.regex` property to the docs
### Why are the changes needed?
The property referred by `spark.sql.redaction.string.regex` description as its default value
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Not needed for docs
--
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] github-actions[bot] closed pull request #33740: [SPARK-36510][DOCS] Add spark.redaction.string.regex property to the docs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #33740:
URL: https://github.com/apache/spark/pull/33740
--
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] AmplabJenkins commented on pull request #33740: [SPARK-36510][DOCS] Add spark.redaction.string.regex property to the docs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33740:
URL: https://github.com/apache/spark/pull/33740#issuecomment-898764723
Can one of the admins verify this patch?
--
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] github-actions[bot] commented on pull request #33740: [SPARK-36510][DOCS] Add spark.redaction.string.regex property to the docs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #33740:
URL: https://github.com/apache/spark/pull/33740#issuecomment-983144183
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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 change in pull request #33740: [SPARK-36510][DOCS] Add spark.redaction.string.regex property to the docs
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #33740:
URL: https://github.com/apache/spark/pull/33740#discussion_r689018185
##########
File path: docs/configuration.md
##########
@@ -659,6 +659,16 @@ Apart from these, the following properties are also available, and may be useful
</td>
<td>2.1.2</td>
</tr>
+<tr>
+ <td><code>spark.redaction.string.regex</code></td>
+ <td>(none)</td>
+ <td>
+ Regex to decide which parts of strings produced by Spark contain sensitive
+ information. When this regex matches a string part, that string part is replaced by a
+ dummy value. This is currently used to redact the output of SQL explain commands.
Review comment:
Hm, sounds like it's currently only used by SQL's config fallback value, and we already documented `spark.sql.redaction.string.regex`. I wouldn't document this one alone for now as it's unlikely set by users at this moment.
--
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] dnskr commented on a change in pull request #33740: [SPARK-36510][DOCS] Add spark.redaction.string.regex property to the docs
Posted by GitBox <gi...@apache.org>.
dnskr commented on a change in pull request #33740:
URL: https://github.com/apache/spark/pull/33740#discussion_r689081057
##########
File path: docs/configuration.md
##########
@@ -659,6 +659,16 @@ Apart from these, the following properties are also available, and may be useful
</td>
<td>2.1.2</td>
</tr>
+<tr>
+ <td><code>spark.redaction.string.regex</code></td>
+ <td>(none)</td>
+ <td>
+ Regex to decide which parts of strings produced by Spark contain sensitive
+ information. When this regex matches a string part, that string part is replaced by a
+ dummy value. This is currently used to redact the output of SQL explain commands.
Review comment:
Yes, you are right. Looks like `STRING_REDACTION_PATTERN ` (`spark.redaction.string.regex`) usage has been removed in [commit#2831571](https://github.com/apache/spark/commit/2831571) in Dec 19, 2017. So it is not used in source code anymore rather than fallback value for `spark.sql.redaction.string.regex` property.
I found having this property in `spark.sql.redaction.string.regex` description confusing because it creates a feeling that `spark.redaction.string.regex` is needed to redact sensetive data in non-SQL places. It might confuse others as well.
Would it be a good idea to remove it from `spark.sql.redaction.string.regex` description to avoid this misunderstanding?
Also in source code we can mark `spark.redaction.string.regex` as `Deprecated` or just add a note that it is not supposed to be set by users.
What do you think?
--
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 change in pull request #33740: [SPARK-36510][DOCS] Add spark.redaction.string.regex property to the docs
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #33740:
URL: https://github.com/apache/spark/pull/33740#discussion_r693418067
##########
File path: docs/configuration.md
##########
@@ -659,6 +659,16 @@ Apart from these, the following properties are also available, and may be useful
</td>
<td>2.1.2</td>
</tr>
+<tr>
+ <td><code>spark.redaction.string.regex</code></td>
+ <td>(none)</td>
+ <td>
+ Regex to decide which parts of strings produced by Spark contain sensitive
+ information. When this regex matches a string part, that string part is replaced by a
+ dummy value. This is currently used to redact the output of SQL explain commands.
Review comment:
I am not sure on this for now. If we happen to add the redaction on other places not in core, `spark.redaction.string.regex` could make sense. We won't necessarily have to deprecate it - I would just prefer to leave it undocumented because semantically it looks fine now although the configuration is virtually useless at this moment.
--
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] dnskr commented on a change in pull request #33740: [SPARK-36510][DOCS] Add spark.redaction.string.regex property to the docs
Posted by GitBox <gi...@apache.org>.
dnskr commented on a change in pull request #33740:
URL: https://github.com/apache/spark/pull/33740#discussion_r689081057
##########
File path: docs/configuration.md
##########
@@ -659,6 +659,16 @@ Apart from these, the following properties are also available, and may be useful
</td>
<td>2.1.2</td>
</tr>
+<tr>
+ <td><code>spark.redaction.string.regex</code></td>
+ <td>(none)</td>
+ <td>
+ Regex to decide which parts of strings produced by Spark contain sensitive
+ information. When this regex matches a string part, that string part is replaced by a
+ dummy value. This is currently used to redact the output of SQL explain commands.
Review comment:
Yes, you are right. Looks like `STRING_REDACTION_PATTERN ` (`spark.redaction.string.regex`) usage has been removed in [commit#2831571](https://github.com/apache/spark/commit/2831571) in Dec 19, 2017. So it is not used in source code anymore rather than fallback value for `spark.sql.redaction.string.regex` property.
I found having this property in `spark.sql.redaction.string.regex` description confusing because it creates a feeling that `spark.redaction.string.regex` is needed to redact sensetive data in non-SQL places. It might confuse others as well.
Would it be a good idea to remove it from `spark.sql.redaction.string.regex` description to avoid this misunderstanding? Also in source code we can mark `spark.redaction.string.regex` as `Deprecated` or just add a note that it is not supposed to be set by users.
What do you think?
--
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