Inconsistent behaviour: multiple --inputs fail, multiple --input-urls do not

[enricorotundo]

This fails:

> bacalhau docker run \
    --inputs bafybeig3xawqpx2i2s6ct5nmfavmm5avyw5h364gzslgrmszgcufpf7zz4 \
    --inputs bafybeig63whfqyuvwqqrp5456fl4anceju24ttyycexef3k5eurg5uvrq4 \
    --wait \
    ubuntu ls /inputs
...
    Stderr:
      Error response from daemon: Duplicate mount point: /inputs

This runs but ignores the second input-urls:

❯ bacalhau docker run \
    --input-urls https://gist.githubusercontent.com/enricorotundo/4c7f158f20ae4997e710e66531b8b54f/raw/ee5228ea06117a6ae97a98fed4ae7b1f1776d564/hepscore-new.md \
    --input-urls https://gist.githubusercontent.com/enricorotundo/990f0ad01a50d08dfb580e4ad404870e/raw/aa6934257351a0da93f1e740c72f27128590cebc/foo_data.txt \
    --wait \
    ubuntu ls /inputs
...
    Stdout:
      foo_data.txt

Tested on v0.3.6

[kcmuspace]

Hello @simonwo. I would like to work on this issue.

I'm wondering if the desired behavior is accepting both inputs or making one flag consistent with the other.

Also, Could I have any background information/any pointers related to this bug? thank you so much :)

[enricorotundo]

hi @kcmuspace IMO the cli should accept 1+ --inputs (e.g. --inputs CID0 --inputs CID1 --inputs CID2) likewise --input-urls (and I believe --input-volumes too). With the caveat that --inputs always targets /inputs mount point, while the other two flags require a target mount point path; this needs to be handled properly.

[thetechnocrat-dev]

@kcmuspace @enricorotundo, multiple CIDS would help us out at LabDAO. Just want to check in on timelines to start this ticket and how I can help get this over the finish line.

[enricorotundo]

hey @thetechnocrat-dev feel free to jump on this one!

[simonwo]

So I think we need to provide some clarification on what the job to be done here is. Currently:

• Using multiple --inputs results in a (server-side) error
• Using multiple --input-urls results in no error, but missing data

So the scope of this ticket is to ensure that both of those paths return an error, ideally on the client side before submission. So @kcmuspace please go ahead and pick this up if it sounds interesting.

@thetechnocrat-dev You can already use multiple CIDs using the --input-volumes flag, which can be specified multiple times. See https://docs.bacalhau.org/all-flags/#docker-run

[enricorotundo]

@simonwo WDYT about allowing multiple --input-urls ?

[simonwo]

@simonwo WDYT about allowing multiple --input-urls ?

I think we do want to support multiple input URLs in the CLI (we alrady support it in the job spec!). The problem with the --input-urls flag is that it tries to put the data in the same place (/inputs) for each URL.

I think ultimately we need the user to specify unique volume names for each input URL, as if they were using --input-volumes. We could either do that by allowing users to put a :volume-name on the end of the URL passed to --input-urls (not sure if that is ambiguous to parse?):

bacalhau docker run ... --input-urls https://gogol.com:/inputs --input-urls https://gitlove.com:/inputs2

Or we could just make --input-volumes auto-detect a CID or URL...

bacalhau docker run ... -v https://google.com/:/inputs -v Qmycid12345:/inputs2

WDYT?

[enricorotundo]

Or we could just make --input-volumes auto-detect a CID or URL...

I like this idea. Similarly, --inputs should accept URL too?

So the behaviour would be:

• --inputs ANYTHING-SUPPORTED mount ANYTHING-SUPPORTED into /inputs. Only one /inputs is supported.
• --input-volumes ANYTHING-SUPPORTED:target-mount mount ANYTHING-SUPPORTED into target-mount. 1+ --input-volumes are supported.

How about that?

[simonwo]

If we're going to change it, why not go one step further and reduce it down to one flag which can optionally take a target mount point? Let's explore that idea.

• ANYTIHNG_SUPPORTED means:
  • An IPFS CID (anything that successfully parses in a call to Cid.parse)
  • A local filesystem path
  • A URL (the URL may support certain schemes that result in different storage types e.g. s3://)
• --inputs ANYTHING-SUPPORTED:target-mount mounts ANYTHING-SUPPORTED into target-mount.
• --inputs ANYTHING-SUPPORTED mounts ANYTHING-SUPPORTED into /inputs.
• It is an error for two storagespecs to mount to the same place.
  • As a result, it is an error to use --inputs without a target-mount more than once.

What do you think?

I think the reason that this wasn't already done is because URLs can of course contain colons, and it's ambiguous whether that will be a target mount or part of the URL, e.g. with --inputs https://en.wikipedia.org/wiki/Template:Welcome, it's not clear whether Welcome is a target-mount of part of the URL. However, URLs support percent-encoding, so a way out of this is just to require that colons in the path segment are percent-encoded (or people can just use a job spec).

There is another colon in the URL though, at the start in the protocol section.

I think this therefore would require parsing logic that looks like:

1. Split the flag on the last colon, and try parsing the first part as a CID, file or URL and the last part as a target-mount.
2. If this fails, try parsing the whole flag as a CID, file or URL and assume the user is targeting /inputs.

With a simple example of parsing https://example.com, this looks like:

1. Parse https as the source and //example.com as the target mount.
   • https is not a valid CID, hopefully not an existant file, and is not a valid URL
   • //example.com is not a valid mount point (is this true? – it might be fine on Windows?)
   • So failed.
2. Parse https://example.com as the source and "" as the target mount.
   • It is a valid URL, so ok.
   • Empty target mount defaults to /inputs, so ok.

... this does not really spark joy, does it? It fails ungracefully in the situation where there are oddly named files on the local filesystem, like https, and certainly is a bit complex to reason about.

So I think I agree with your simpler approach – by telling the client whether it should expect a target-mount component or not, we can do away with trying to deal with the ambiguity.

TL;DR: we should do what Enrico suggested.

[kcmuspace]

@simonwo I would like to try to solve the issue.

So, for summary and clarification. like @enricorotundo said

• --inputs ANYTHING-SUPPORTED mount ANYTHING-SUPPORTED into /inputs. Only one /inputs is supported. So this would mean only one --inputs flag

• --input-volumes ANYTHING-SUPPORTED:target-mount mount ANYTHING-SUPPORTED into target-mount. 1+ --input-volumes are supported.

How about --input-urls ? Should that be deleted and --inputs would handle the behavior of --input-urls

[enricorotundo]

How about --input-urls ? Should that be deleted and --inputs would handle the behavior of --input-urls

I think so yeah. @kcmuspace

[enricorotundo]

hi @kcmuspace #1971 overlaps with this 🤔 . Did you have the chance to have a go?

[kcmuspace]

Hi @enricorotundo . I've been trying but unfortunately I've made minimal progress(I think due to my unfamiliarity with the codebase). I can see that this issue is more important( and more complex) than I originally thought evidenced by the linked issues. Would this requirement need to be completed ASAP?

I think this could take me a while. I understand if someone more experienced wants to take over. I can work on something other than this.

[enricorotundo]

This PR #1936 adds yet a new input type, take a look at its status when diving into this issue

Hi @enricorotundo . I've been trying but unfortunately I've made minimal progress(I think due to my unfamiliarity with the codebase). I can see that this issue is more important( and more complex) than I originally thought evidenced by the linked issues. Would this requirement need to be completed ASAP?

@kcmuspace not necessarily ASAP actually it's totally up to you if you want to contribute. But feel free to seek help on Slack if this is still in the works :)

[aronchick]

Investigate if still relevant

[thetechnocrat-dev]

For what it is worth, we have been successfully passing multiple CIDs through our Go integration at LabDAO for while now.