[GitHub] spark pull request #21158: [SPARK-23850][sql] Add separate config for SQL op...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/21158


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21158: [SPARK-23850][sql] Add separate config for SQL op...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/21158#discussion_r185143980
  
--- Diff: 
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -342,7 +342,7 @@ package object config {
 "a property key or value, the value is redacted from the 
environment UI and various logs " +
 "like YARN and event logs.")
   .regexConf
-  .createWithDefault("(?i)secret|password|url|user|username".r)
+  .createWithDefault("(?i)secret|password".r)
--- End diff --

URIs could contain the access keys and usernames could have potentially 
personal identifiable information. Could we do not change the default?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21158: [SPARK-23850][sql] Add separate config for SQL op...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/21158#discussion_r185142687
  
--- Diff: 
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -342,7 +342,7 @@ package object config {
 "a property key or value, the value is redacted from the 
environment UI and various logs " +
 "like YARN and event logs.")
   .regexConf
-  .createWithDefault("(?i)secret|password|url|user|username".r)
+  .createWithDefault("(?i)secret|password".r)
--- End diff --

Normally, making it stricter is not a big deal. However, if we want to 
relax it, this could be a big deal since users might expect these variables 
have been hided by default. cc the original reviewers and author @onursatici 
@ash211 @hvanhovell @jiangxb1987 


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21158: [SPARK-23850][sql] Add separate config for SQL op...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/21158#discussion_r184152897
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala ---
@@ -225,7 +225,7 @@ class QueryExecution(val sparkSession: SparkSession, 
val logical: LogicalPlan) {
* Redact the sensitive information in the given string.
*/
   private def withRedaction(message: String): String = {
-Utils.redact(sparkSession.sessionState.conf.stringRedationPattern, 
message)
+Utils.redact(sparkSession.sessionState.conf.stringRedactionPattern, 
message)
--- End diff --

Thanks for fixing this typo


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21158: [SPARK-23850][sql] Add separate config for SQL op...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/21158#discussion_r184152311
  
--- Diff: 
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -342,7 +342,7 @@ package object config {
 "a property key or value, the value is redacted from the 
environment UI and various logs " +
 "like YARN and event logs.")
   .regexConf
-  .createWithDefault("(?i)secret|password|url|user|username".r)
+  .createWithDefault("(?i)secret|password".r)
--- End diff --

Not sure what you're trying to imply? The default is redacting too much. 
That needs to be fixed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21158: [SPARK-23850][sql] Add separate config for SQL op...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/21158#discussion_r184151995
  
--- Diff: 
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -342,7 +342,7 @@ package object config {
 "a property key or value, the value is redacted from the 
environment UI and various logs " +
 "like YARN and event logs.")
   .regexConf
-  .createWithDefault("(?i)secret|password|url|user|username".r)
+  .createWithDefault("(?i)secret|password".r)
--- End diff --

If the users want to see the values, they can change the default by 
themselves, right?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21158: [SPARK-23850][sql] Add separate config for SQL op...

[vanzin]

GitHub user vanzin opened a pull request:

https://github.com/apache/spark/pull/21158

[SPARK-23850][sql] Add separate config for SQL options redaction.

The old code was relying on a core configuration and extended its
default value to include things that redact desired things in the
app's environment. Instead, add a SQL-specific option for which
options to redact, and apply both the core and SQL-specific rules
when redacting the options in the save command.

This is a little sub-optimal since it adds another config, but it
retains the current default behavior.

While there I also fixed a typo and a couple of minor config API
usage issues in the related redaction option that SQL already had.

Tested with existing unit tests, plus checking the env page on
a shell UI.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vanzin/spark SPARK-23850

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/21158.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #21158


commit deb299b0211977735f6a830f763d4b9c37f5960c
Author: Marcelo Vanzin 
Date:   2018-04-24T22:52:58Z

[SPARK-23850][sql] Add separate config for SQL options redaction.

The old code was relying on a core configuration and extended its
default value to include things that redact desired things in the
app's environment. Instead, add a SQL-specific option for which
options to redact, and apply both the core and SQL-specific rules
when redacting the options in the save command.

This is a little sub-optimal since it adds another config, but it
retains the current default behavior.

While there I also fixed a typo and a couple of minor config API
usage issues in the "sistes" redaction option that SQL already had.

Tested with existing unit tests, plus checking the env page on
a shell UI.




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org