Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
(I'll open a separate PR for 2.2 to run tests before pushing, since there
are conflicts.)
---
-
To unsubscribe, e-mail:
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
Merging to master / 2.3 / 2.2.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands,
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21158
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90750/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21158
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21158
**[Test build #90750 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90750/testReport)**
for PR 21158 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21158
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21158
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3309/
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21158
**[Test build #90750 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90750/testReport)**
for PR 21158 at commit
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
retest this please
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user onursatici commented on the issue:
https://github.com/apache/spark/pull/21158
I don't have any strong opinions about usernames, so merging this as it
stays is fine by me
---
-
To unsubscribe, e-mail:
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
I plan to push this tomorrow by EOD unless I hear strong opposition, and
merge it to 2.3 and 2.2 (where the original change was also backported). I want
to avoid having another release out with the
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
That's fine, but I'm not going to wait indefinitely for their feedback.
They've already been pinged multiple times here and on jira to comment on this
issue.
---
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21158
I would like to hear more inputs from the reviewers of the original PR. cc
@onursatici @ash211 @hvanhovell @jiangxb1987
---
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
Last ping before I assume everybody is ok with the patch.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
ping again
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
ping
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
> To keep all the previous behaviour, SQL_OPTIONS_REDACTION_PATTERN can
include user
User names, unlike passwords, are useful for debugging. And they're not
meant to be secret. They're
Github user onursatici commented on the issue:
https://github.com/apache/spark/pull/21158
I think this PR is fine given it preserves the redaction of URL's. That was
indeed the main reason why url's were in the default redaction pattern in the
previous PR.
To keep all the
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
> Not all the Spark end users are allowed to access the command lines and
run ls
Come on. That is just an example. You surely are smart enough to think of
another example where you can see
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21158
@vanzin That really depends on how the users deploy Spark. Not all the
Spark end users are allowed to access the command lines and run `ls`, but they
are allowed to read Spark UI in almost all
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
> usernames could have potentially personal identifiable information
User names are not sensitive information. There's a ton of places where you
can see them outside of the Spark UI (have
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
@gatorsmile seriously, why are you focusing on the fact that I'm changing
that default value when I wrote a bunch of code to actually maintain the URL
redaction on the SQL plans, which was the
Github user tgravescs commented on the issue:
https://github.com/apache/spark/pull/21158
Also, Spark by default is shipped with not secure settings. Meaning
spark.acls.enable is false and spark.authenticate is false. I see no reason to
make the redact configs more strict then our
Github user tgravescs commented on the issue:
https://github.com/apache/spark/pull/21158
I disagree on this. I find it a regression that you blocked url's and user
names in the first place. Showing these things have been a feature in spark
for a long time. The only security issue
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21158
The default value introduced by SPARK-22479 was already part of Apache
Spark 2.3. It is hard to say this is a regression, since URIs could contain the
access keys and usernames could have
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
@gatorsmile is there anything else you're looking for here?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
Also note that Tom's comment above is not 100% accurate - this change
*does* redact the JDBC URL on the SQL side, for reasons I described in my
comments on the bug. That's the whole reason why the
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
But the main thing is that the change modified the previous behavior which
was *not* to redact these things in the previous places where redaction
occurred. So, in a way, this change is fixing a
Github user tgravescs commented on the issue:
https://github.com/apache/spark/pull/21158
the original change was merged into 2.3.0, see
https://issues.apache.org/jira/browse/SPARK-22479. We are keeping the
behavior of that fix and that is to not show the credentials of the jdbc
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21158
Is the default `(?i)secret|password|url|user|username".r` merged to the
released branches? If the default is not in the previously release, I am fine
to change it back.
---
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
> We are making a behavior change here.
We are *not*. That's the whole reason why I'm adding the SQL-specific
option to extend the behavior of the core options.
If I just wanted to
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21158
The confs in the core modules are not part of the outputs of SQL statement
`SET -v`, which only outputs the confs in Spark SQL.
We are making a behavior change here. I am not confident
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
Ping @gatorsmile
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21158
> I was looking for documentation on these sql confs
That's part of this fix, if you're talking about the old config. Since it
was using `ConfigBuilder` directly instead of `buildConf`, it
Github user tgravescs commented on the issue:
https://github.com/apache/spark/pull/21158
Looks good. I didn't have a jdbc connection handy to test with.
I was looking for documentation on these sql confs and I don't see them in
the SET -v output, did we decide not to
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21158
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/89855/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21158
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21158
**[Test build #89855 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89855/testReport)**
for PR 21158 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21158
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2679/
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21158
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21158
**[Test build #89855 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89855/testReport)**
for PR 21158 at commit
41 matches
Mail list logo
