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

Pipe API doesn't properly handle multiple arguments and spaces #1909

Closed
benwbooth opened this Issue Feb 12, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@benwbooth
Copy link

commented Feb 12, 2018

The pipe API command does a simple split on spaces to split into multiple arguments. This will fail if any of the commands are arguments include spaces in them. There is currently no workaround for this. The underlying java ProcessBuilder API allows running commands with multiple arguments by passing in a list of parameters. The pipe API should thus also allow passing in a list of parameters instead of a single string. This would fix the argument separation and escaping issues. So the new interface would look like:

  def pipe[X, Y <: GenomicRDD[X, Y], V <: InFormatter[T, U, V]](
    cmd: Seq[String],
    files: Seq[String] = Seq.empty,
@heuermh

This comment has been minimized.

Copy link
Member

commented Feb 12, 2018

Thanks for reporting this, @benwbooth!

The pipe API command does a simple split on spaces to split into multiple arguments. This will fail if any of the commands are arguments include spaces in them.

Yes, this is an issue.

cmd: Seq[String], ...

This feels correct to me, though we also need to support the pipe API via Java, Python, and R, so the fix will need to propagate to those supporting methods.

Note in Cannoli we want to support Docker and Singularity over the pipe API, and perhaps a container-aware ProcessBuilder API developed there might be useful to surface in ADAM. Would you find that helpful?

@heuermh heuermh added this to the 0.24.0 milestone Feb 15, 2018

fnothaft added a commit that referenced this issue Feb 19, 2018

[ADAM-1909] Refactor pipe cmd parameter from String to Seq[String].
Refactor pipe cmd parameter from String to Seq[String]. Resolves #1909.
@heuermh

This comment has been minimized.

Copy link
Member

commented Feb 19, 2018

Thank you for submitting this issue, @benwbooth!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.