HumanCellAtlas / dcp-cli

DEPRECATED - HCA Data Coordination Platform Command Line Interface
https://hca.readthedocs.io/
MIT License
6 stars 8 forks source link

Order of arguments matters for `hca dss download-manifest` #516

Closed chmreid closed 4 years ago

chmreid commented 4 years ago

If running an hca dss download-manifest command, the order of the flags matters.

The problem is that the --manifest flag takes multiple arguments, so if you specify

hca dss download-manifest --manifest X --layout Y

it will interpret Y as an argument to --manifest and fail, but if you specify

hca dss download-manifest --layout Y --manifest X

it will associate Y with the --layout argument and the command will work.

The importance of flag order is not documented in the download-manifest documentation.

hannes-ucsc commented 4 years ago

This must be a bug in the code that derives the argparse configuration from the function signatures. It's entirely possible that this affects other sub commands.

The problem is that the --manifest flag takes multiple arguments

What makes you come to that conclusion? AFAICT, it should be requiring a single argument:

https://github.com/HumanCellAtlas/dcp-cli/blob/22172e69e8d6c9739f361d6c8d30b8f9581f7db5/hca/dss/__init__.py#L256

chmreid commented 4 years ago

The CLI interprets Y as an argument to --manifest when you run

hca dss download-manifest --manifest X --layout Y

which seems to be because the --manifest flag takes multiple arguments. But it could be some other reason.

hannes-ucsc commented 4 years ago

I think you said as much in the original post. My question was as to WHY do you think it takes multiple arguments. Like in: What's the evidence behind your claim? I agree that it appears that --manifest eats the Y argument, but the question remains: Why does it do that? It is certainly not declared to do consume multiple arguments. The only difference between manifest and layout that I see is that layout is declared optional while manifest is mandatory.

chmreid commented 4 years ago

The command that caused this error was something @theathorn was typing on his machine, so it will take some extra time/work to replicate this error on my machine.

chmreid commented 4 years ago

I am unable to reproduce this bug. Both of these commands work okay on my machine:

$ hca dss download-manifest --replica aws --layout bundle --manifest manifest.tsv
$ hca dss download-manifest --replica aws --manifest manifest.tsv --layout bundle

The --manifest flag also accepts one and only one input argument. @theathorn could you try running history | grep download-manifest on your machine to see if the commands that were failing are still in your bash history?

theathorn commented 4 years ago

I was using a Linux VM on my laptop at work, which I won't have access to for weeks..

chmreid commented 4 years ago

In that case, I think we can close this ticket with "cannot reproduce" and re-open if the issue comes up again.