Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove formatPath() that adds ".\" or "./" to the executable. #81

Merged
merged 3 commits into from
May 1, 2019

Conversation

imback82
Copy link
Contributor

@imback82 imback82 commented May 1, 2019

formatPath() adds "." or "." to the .NET executable. This forces the user to specify a full path for the .NET executable passed to spark-submit. For example,

spark-submit --class org.apache.spark.deploy.DotnetRunner microsoft-spark-<version>.jar dotnet HelloSpark.dll

is not allowed, since formatPath() changes dotnet to .\dotnet.exe even if dotnet is in the PATH.

@imback82 imback82 requested review from rapoth and suhsteve May 1, 2019 02:30
@imback82 imback82 self-assigned this May 1, 2019
@imback82 imback82 added the enhancement New feature or request label May 1, 2019
@@ -71,11 +71,6 @@ object DotnetRunner extends Logging {
// Reuse windows-specific formatting in PythonRunner.
dotnetExecutable = PythonRunner.formatPath(resolveDotnetExecutable(driverDir, args(1)))
otherArgs = args.slice(2, args.length)
} else if (new File(args(0)).isDirectory) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this since there is no use case for this.

@@ -200,17 +195,6 @@ object DotnetRunner extends Logging {
resolvedExecutable
}

// When executing in YARN cluster mode, the name of the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that for YARN cluster mode, the user has to ship the executable with dependencies via Zip file and the full path for the executable is constructed from it. Thus, this function is not needed.

Copy link
Contributor

@rapoth rapoth left a comment

Choose a reason for hiding this comment

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

LGTM

@suhsteve
Copy link
Member

suhsteve commented May 1, 2019

LGTM

@imback82 imback82 merged commit 8d88b24 into dotnet:master May 1, 2019
@imback82 imback82 deleted the remove_current_path branch May 1, 2019 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants