[SPARK-32036] Replace references to blacklist/whitelist language with more appropriate terminology, excluding the blacklisting feature. #28874

xkrogen · 2020-06-19T23:47:50Z

What changes were proposed in this pull request?

This PR will remove references to these "blacklist" and "whitelist" terms besides the blacklisting feature as a whole, which can be handled in a separate JIRA/PR.

This touches quite a few files, but the changes are straightforward (variable/method/etc. name changes) and most quite self-contained.

Why are the changes needed?

As per discussion on the Spark dev list, it will be beneficial to remove references to problematic language that can alienate potential community members. One such reference is "blacklist" and "whitelist". While it seems to me that there is some valid debate as to whether these terms have racist origins, the cultural connotations are inescapable in today's world.

Does this PR introduce any user-facing change?

In the test file HiveQueryFileTest, a developer has the ability to specify the system property spark.hive.whitelist to specify a list of Hive query files that should be tested. This system property has been renamed to spark.hive.includelist. The old property has been kept for compatibility, but will log a warning if used. I am open to feedback from others on whether keeping a deprecated property here is unnecessary given that this is just for developers running tests.

How was this patch tested?

Existing tests should be suitable since no behavior changes are expected as a result of this PR.

xkrogen · 2020-06-19T23:48:43Z

@holdenk can you help review?

R/pkg/tests/fulltests/test_sparkSQL.R

core/src/test/scala/org/apache/spark/ui/UISeleniumSuite.scala

holdenk · 2020-06-20T02:30:01Z

Sure, I'm taking this weekend away from coding so I'll get to this early next week.

xkrogen · 2020-06-22T22:10:37Z

Updated to address @venkata91 's comments and fix one class I missed in the hive-thriftserver module.

tgravescs · 2020-06-23T17:24:47Z

ok to test

core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala

docs/streaming-programming-guide.md

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JSONOptions.scala

sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/json/JsonSuite.scala

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQueryFileTest.scala

SparkQA · 2020-06-23T20:17:16Z

Test build #124427 has finished for PR 28874 at commit 5963b54.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

xkrogen · 2020-06-24T16:54:31Z

Updated to resolve whitespace errors and some typos per suggestions by @tgravescs . Thanks!

SparkQA · 2020-06-24T17:09:00Z

Test build #124492 has finished for PR 28874 at commit 32ccda7.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

xkrogen · 2020-06-24T17:15:25Z

I'm not sure why the Jenkins build failed. I see this message:

Using `mvn` from path: /home/jenkins/tools/hudson.tasks.Maven_MavenInstallation/Maven_3.6.3/bin/mvn
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-install-plugin:3.0.0-M1:install (default-cli) on project spark-parent_2.12: ArtifactInstallerException: Failed to install metadata org.apache.spark:spark-parent_2.12/maven-metadata.xml: Could not parse metadata /home/jenkins/.m2/repository/org/apache/spark/spark-parent_2.12/maven-metadata-local.xml: in epilog non whitespace content is not allowed but got > (position: END_TAG seen ...</metadata>\n>... @13:2) -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException

I do not understand how it could be related to my patch, which doesn't touch anything Maven-related. Any suggestions?

holdenk · 2020-06-24T18:12:00Z

Yeah I've been running into that too. I reached out to Shane and he took the worker that has the corrupted m2 cache out of rotation. So if we do "Jenkins retest this please" hopefully the other Jenkin's workers have a better m2 cache :)

holdenk

Thanks for working on this, I'm happy that we are cleaning this up :)

core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala

core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQueryFileTest.scala

SparkQA · 2020-06-24T21:00:37Z

Test build #124497 has finished for PR 28874 at commit 32ccda7.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-06-24T23:30:38Z

Test build #124500 has finished for PR 28874 at commit c3214de.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala

SparkQA · 2020-06-26T19:20:18Z

Test build #124552 has finished for PR 28874 at commit 09ec2e8.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

tgravescs

looks good to me. @holdenk any other comments?

srowen

Looks like it's all internal changes, with no behavior changes, so seems fine to me.

core/src/main/scala/org/apache/spark/deploy/rest/RestSubmissionClient.scala

srowen

Looks OK to me, any more comments? I can merge if not.
Let me run tests one more time to be sure.

srowen · 2020-07-11T18:40:04Z

Jenkins test this please

SparkQA · 2020-07-11T22:12:46Z

Test build #125689 has finished for PR 28874 at commit 09ec2e8.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2020-07-12T14:54:20Z

LGTM but we have a merge conflict now

tgravescs · 2020-07-13T14:03:06Z

@xkrogen can you up merge?

xkrogen · 2020-07-13T16:45:24Z

Thanks a lot for the new eyes @srowen and @tgravescs ! I've just pushed up a resolution to the conflict.

SparkQA · 2020-07-13T20:27:22Z

Test build #125780 has finished for PR 28874 at commit 5cd7d81.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

tgravescs · 2020-07-14T13:29:49Z

@xkrogen unfortunately it appears there is another conflict could you update again?

… more appropriate terminology, excluding the blacklisting feature.

…nt, rename one variable based on feedback.

…the initial commit.

…rceDownloadSchemas to forceDownloadSchemes

xkrogen · 2020-07-14T15:31:56Z

Playing a game of whack-a-mole here :) Thanks for the heads up @tgravescs . Pushed up another conflict resolution.

SparkQA · 2020-07-14T18:35:41Z

Test build #125843 has finished for PR 28874 at commit 1ba58fb.

This patch fails PySpark pip packaging tests.
This patch merges cleanly.
This patch adds no public classes.

xkrogen · 2020-07-14T18:39:11Z

I think the pip packaging test failure is not related to my changes, but I cannot really tell.

Installing collected packages: py4j, pyspark
  Found existing installation: py4j 0.10.9
    Uninstalling py4j-0.10.9:
      Successfully uninstalled py4j-0.10.9
  Found existing installation: pyspark 3.1.0.dev0
Exception:
Traceback (most recent call last):
  File "/home/anaconda/envs/py36/lib/python3.6/site-packages/pip/_internal/cli/base_command.py", line 179, in main
    status = self.run(options, args)
  File "/home/anaconda/envs/py36/lib/python3.6/site-packages/pip/_internal/commands/install.py", line 393, in run
    use_user_site=options.use_user_site,
  File "/home/anaconda/envs/py36/lib/python3.6/site-packages/pip/_internal/req/__init__.py", line 50, in install_given_reqs
    auto_confirm=True
  File "/home/anaconda/envs/py36/lib/python3.6/site-packages/pip/_internal/req/req_install.py", line 816, in uninstall
    uninstalled_pathset = UninstallPathSet.from_dist(dist)
  File "/home/anaconda/envs/py36/lib/python3.6/site-packages/pip/_internal/req/req_uninstall.py", line 505, in from_dist
    '(at %s)' % (link_pointer, dist.project_name, dist.location)
AssertionError: Egg-link /home/jenkins/workspace/SparkPullRequestBuilder@2/python does not match installed location of pyspark (at /home/jenkins/workspace/SparkPullRequestBuilder/python)
Cleaning up temporary directory - /tmp/tmp.CjjxmjJmax
[error] running /home/jenkins/workspace/SparkPullRequestBuilder/dev/run-pip-tests ; received return code 2

tgravescs · 2020-07-14T18:44:37Z

all GitHub action tests passed so I think we are good. I'll kick it once more just to check.

tgravescs · 2020-07-14T18:44:42Z

test this please

SparkQA · 2020-07-14T22:08:20Z

Test build #125852 has finished for PR 28874 at commit 1ba58fb.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

tgravescs · 2020-07-15T15:11:42Z

@xkrogen unfortunately think we are still playing wack a mole, it doesn't seem mergable, could you upmerge again and I'll try to watch closely and get merged.

xkrogen · 2020-07-15T15:23:10Z

@tgravescs GitHub seems to be reporting no conflicts, and when I just put together a rebase, there were also no conflicts. Can you check again?

I'm happy to push up a rebased branch, but don't want to have to wait for another test cycle if it's not necessary :)

tgravescs · 2020-07-15T16:40:00Z

weird, let me try to commit again, it was reporting couldn't do it, but I also saw this PR was stuck and wouldn't say if it was mergeable

tgravescs · 2020-07-15T16:41:19Z

thanks @xkrogen merged to master

xkrogen · 2020-07-15T16:49:39Z

Thanks a lot @tgravescs !

HyukjinKwon · 2020-07-16T01:32:49Z

python/pyspark/cloudpickle.py

-    PY2_CLASS_DICT_BLACKLIST = (PY2_METHOD_WRAPPER_TYPE,
-                                PY2_WRAPPER_DESCRIPTOR_TYPE)
+    PY2_CLASS_DICT_SKIP_PICKLE_METHOD_TYPE = (PY2_METHOD_WRAPPER_TYPE,
+                                              PY2_WRAPPER_DESCRIPTOR_TYPE)


Hey let's dont change this file but keep as the release (and fix their release and port it together). It's the exact copy of cloudpickle release. We just port their fixes and dont have conflicts here for management purpose. Linters skip this file too and we had to put a lot of efforts to resolve the conflicts here before.

I am going to upgrade this file at #29114. Seems that release version doesn't have this issue.

ok so since you are upgrading, we are good to leave it as is, correct? Your merge will fix it up.

Yup, I think we're all good

Thanks for addressing this @HyukjinKwon ! I avoided making changes in other files that I could tell were direct copies (e.g. stuff in hive-thriftserver) but did not realize this was one of those cases.

HyukjinKwon · 2020-07-16T01:45:11Z

+1 the changes look good

probot-autolabeler bot added BUILD CORE DOCS EXAMPLES MESOS PYTHON R SQL WEB UI YARN labels Jun 19, 2020

venkata91 reviewed Jun 20, 2020

View reviewed changes

R/pkg/tests/fulltests/test_sparkSQL.R Outdated Show resolved Hide resolved

core/src/test/scala/org/apache/spark/ui/UISeleniumSuite.scala Show resolved Hide resolved

tgravescs reviewed Jun 23, 2020

View reviewed changes

xkrogen force-pushed the xkrogen-SPARK-32036-rename-blacklists branch from 5963b54 to 32ccda7 Compare June 24, 2020 16:53

holdenk reviewed Jun 24, 2020

View reviewed changes

tgravescs reviewed Jun 25, 2020

View reviewed changes

core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala Show resolved Hide resolved

tgravescs approved these changes Jun 29, 2020

View reviewed changes

srowen reviewed Jun 29, 2020

View reviewed changes

core/src/main/scala/org/apache/spark/deploy/rest/RestSubmissionClient.scala Show resolved Hide resolved

srowen reviewed Jul 9, 2020

View reviewed changes

xkrogen force-pushed the xkrogen-SPARK-32036-rename-blacklists branch from 09ec2e8 to 5cd7d81 Compare July 13, 2020 16:44

xkrogen added 5 commits July 14, 2020 08:30

[SPARK-32036] Replace references to blacklist/whitelist language with…

48b07f9

… more appropriate terminology, excluding the blacklisting feature.

Fix compile issue in ThriftServerQueryTestSuite, add additional comme…

df569e0

…nt, rename one variable based on feedback.

Address Thomas Graves' comments except whitespace which was fixed in …

912baf4

…the initial commit.

Rename FsHistoryProvider ignoreList to inaccessibleList and rename fo…

c9e59a0

…rceDownloadSchemas to forceDownloadSchemes

Fix definition of FsHistoryProvider.isAccessible and relevant tests

1ba58fb

xkrogen force-pushed the xkrogen-SPARK-32036-rename-blacklists branch from 5cd7d81 to 1ba58fb Compare July 14, 2020 15:31

asfgit closed this in cf22d94 Jul 15, 2020

xkrogen deleted the xkrogen-SPARK-32036-rename-blacklists branch July 15, 2020 17:11

HyukjinKwon reviewed Jul 16, 2020

View reviewed changes

[SPARK-32036] Replace references to blacklist/whitelist language with more appropriate terminology, excluding the blacklisting feature. #28874

[SPARK-32036] Replace references to blacklist/whitelist language with more appropriate terminology, excluding the blacklisting feature. #28874

Uh oh!

Conversation

xkrogen commented Jun 19, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

xkrogen commented Jun 19, 2020

Uh oh!

Uh oh!

Uh oh!

holdenk commented Jun 20, 2020

Uh oh!

xkrogen commented Jun 22, 2020

Uh oh!

tgravescs commented Jun 23, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Jun 23, 2020

Uh oh!

xkrogen commented Jun 24, 2020

Uh oh!

SparkQA commented Jun 24, 2020

Uh oh!

xkrogen commented Jun 24, 2020

Uh oh!

holdenk commented Jun 24, 2020

Uh oh!

holdenk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Jun 24, 2020

Uh oh!

SparkQA commented Jun 24, 2020

Uh oh!

Uh oh!

SparkQA commented Jun 26, 2020

Uh oh!

tgravescs left a comment

Choose a reason for hiding this comment

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

srowen commented Jul 11, 2020

Uh oh!

SparkQA commented Jul 11, 2020

Uh oh!

srowen commented Jul 12, 2020

Uh oh!

tgravescs commented Jul 13, 2020

Uh oh!

xkrogen commented Jul 13, 2020

Uh oh!

SparkQA commented Jul 13, 2020

Uh oh!

tgravescs commented Jul 14, 2020

Uh oh!

xkrogen commented Jul 14, 2020

Uh oh!

SparkQA commented Jul 14, 2020

Uh oh!

xkrogen commented Jul 14, 2020

Uh oh!

tgravescs commented Jul 14, 2020

Uh oh!

HyukjinKwon Jul 16, 2020 •

edited

Loading