14880: Fix argument parsing issues

arv-keepdocker passes through some flags to arv-put.  The strategy it
uses is to remove known flags and pass through unknown flags. However
it doesn't know if an unknown flag takes a value or not, so it treats
the values as a positional argument rather than the value
corresponding to the unknown flag.

* Move positional arguments from "keepdocker_parser" to "arg_parser"
* Strip positional arguments (image name, tag) from arguments before
  parsing known keepdocker_parser flags.
* Additionally move a couple of flags that were mistakenly added to
  put.run_opts (and thus appearing in the arv-keepdocker help despite
  making no sense) that should have been in put.upload_opts

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>

diff --git a/sdk/python/arvados/commands/keepdocker.py b/sdk/python/arvados/commands/keepdocker.py
index e596e669156f9014ce5b9ad66d05fc2ba561bb3a..019b949e53745612a03455946c3702af2373f422 100644 (file)
--- a/sdk/python/arvados/commands/keepdocker.py
+++ b/sdk/python/arvados/commands/keepdocker.py
@@ -64,13 +64,6 @@ _group.add_argument(
     '--no-pull', action='store_false', dest='pull',
     help="Use locally installed image only, don't pull image from Docker registry (default)")
 
-keepdocker_parser.add_argument(
-    'image', nargs='?',
-    help="Docker image to upload: repo, repo:tag, or hash")
-keepdocker_parser.add_argument(
-    'tag', nargs='?',
-    help="Tag of the Docker image to upload (default 'latest'), if image is given as an untagged repo name")
-
 # Combine keepdocker options listed above with run_opts options of arv-put.
 # The options inherited from arv-put include --name, --project-uuid,
 # --progress/--no-progress/--batch-progress and --resume/--no-resume.
@@ -78,6 +71,13 @@ arg_parser = argparse.ArgumentParser(
         description="Upload or list Docker images in Arvados",
         parents=[keepdocker_parser, arv_put.run_opts, arv_cmd.retry_opt])
 
+arg_parser.add_argument(
+    'image', nargs='?',
+    help="Docker image to upload: repo, repo:tag, or hash")
+arg_parser.add_argument(
+    'tag', nargs='?',
+    help="Tag of the Docker image to upload (default 'latest'), if image is given as an untagged repo name")
+
 class DockerError(Exception):
     pass
 
@@ -492,6 +492,9 @@ def main(arguments=None, stdout=sys.stdout, install_sig_handlers=True, api=None)
 
         # Call arv-put with switches we inherited from it
         # (a.k.a., switches that aren't our own).
+        if arguments is None:
+            arguments = sys.argv[1:]
+        arguments = [i for i in arguments if i not in (args.image, args.tag, image_repo_tag)]
         put_args = keepdocker_parser.parse_known_args(arguments)[1]
 
         if args.name is None:

diff --git a/sdk/python/arvados/commands/put.py b/sdk/python/arvados/commands/put.py
index 54fa356d3a4537364ab770175addc742ade4826d..b219def55381d31726d54bb0cf299b355cfc0032 100644 (file)
--- a/sdk/python/arvados/commands/put.py
+++ b/sdk/python/arvados/commands/put.py
@@ -154,18 +154,7 @@ On high latency installations, using a greater number will improve
 overall throughput.
 """)
 
-run_opts = argparse.ArgumentParser(add_help=False)
-
-run_opts.add_argument('--project-uuid', metavar='UUID', help="""
-Store the collection in the specified project, instead of your Home
-project.
-""")
-
-run_opts.add_argument('--name', help="""
-Save the collection with the specified name.
-""")
-
-run_opts.add_argument('--exclude', metavar='PATTERN', default=[],
+upload_opts.add_argument('--exclude', metavar='PATTERN', default=[],
                       action='append', help="""
 Exclude files and directories whose names match the given glob pattern. When
 using a path-like pattern like 'subdir/*.txt', all text files inside 'subdir'
@@ -177,6 +166,28 @@ the given input directory, you can use a pattern like './exclude_this.gif'.
 You can specify multiple patterns by using this argument more than once.
 """)
 
+_group = upload_opts.add_mutually_exclusive_group()
+_group.add_argument('--follow-links', action='store_true', default=True,
+                    dest='follow_links', help="""
+Follow file and directory symlinks (default).
+""")
+_group.add_argument('--no-follow-links', action='store_false', dest='follow_links',
+                    help="""
+Do not follow file and directory symlinks.
+""")
+
+
+run_opts = argparse.ArgumentParser(add_help=False)
+
+run_opts.add_argument('--project-uuid', metavar='UUID', help="""
+Store the collection in the specified project, instead of your Home
+project.
+""")
+
+run_opts.add_argument('--name', help="""
+Save the collection with the specified name.
+""")
+
 _group = run_opts.add_mutually_exclusive_group()
 _group.add_argument('--progress', action='store_true',
                     help="""
@@ -213,16 +224,6 @@ _group.add_argument('--no-resume', action='store_false', dest='resume',
 Do not continue interrupted uploads from cached state.
 """)
 
-_group = run_opts.add_mutually_exclusive_group()
-_group.add_argument('--follow-links', action='store_true', default=True,
-                    dest='follow_links', help="""
-Follow file and directory symlinks (default).
-""")
-_group.add_argument('--no-follow-links', action='store_false', dest='follow_links',
-                    help="""
-Do not follow file and directory symlinks.
-""")
-
 _group = run_opts.add_mutually_exclusive_group()
 _group.add_argument('--cache', action='store_true', dest='use_cache', default=True,
                     help="""