Skip to content

Conversation

@farhan5900
Copy link
Contributor

What changes were proposed in this pull request?

This PR removes generic shellEscape and put it in specific places. More specifically shell-escape only appName, mainClass, default, and driverConf.

Why are the changes needed?

Changes are needed because we see PySpark jobs fail to launch when 1) run with docker and 2) including --py-files

Does this PR introduce any user-facing change?

No

How was this patch tested?

  • Unit Test

@HyukjinKwon
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Test build #130063 has finished for PR 30112 at commit d5435d2.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34671/

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34671/

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-33199] Mesos Task Failed when pyFiles and docker image option used together [SPARK-33199][MESOS] Mesos Task Failed when pyFiles and docker image option used together Oct 21, 2020
@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Test build #130092 has finished for PR 30112 at commit 30a0459.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34701/

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Test build #130095 has finished for PR 30112 at commit 89e2671.

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

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34701/

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34704/

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Test build #130096 has finished for PR 30112 at commit 8d0d329.

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

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34704/

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34705/

@SparkQA
Copy link

SparkQA commented Oct 21, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34705/

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @farhan5900 .

  1. This PR seems less robust than the original Spark code. For example, this PR changes like the following and injects shellEscape into several places.
- options.map(shellEscape)
+ options
  1. I'm wondering if you try to quote your docker image option at --pyFiles? Is it insufficient for your use cases?

@farhan5900
Copy link
Contributor Author

farhan5900 commented Oct 26, 2020

Hi, @dongjoon-hyun It is basically done to avoid shell-escaping of the $MESOS_SANDBOX used in the case of --py-files with the docker image option (spark.mesos.executor.docker.image).

@github-actions
Copy link

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!

@github-actions github-actions bot added the Stale label Feb 16, 2021
@github-actions github-actions bot closed this Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants