Change arv-put default behavior: write a manifest, even for a single file.
authorTom Clegg <tom@clinicalfuture.com>
Mon, 16 Dec 2013 21:27:14 +0000 (13:27 -0800)
committerTom Clegg <tom@clinicalfuture.com>
Mon, 16 Dec 2013 21:30:21 +0000 (13:30 -0800)
refs #1646

sdk/cli/test/test_arv-put.rb
sdk/python/bin/arv-put

index 5efbfda8298ec03b8dcd1e89ec27bea16ae4798d..5dca976dc11ea9f7c430ef9f676b98a0a0678968 100644 (file)
@@ -25,6 +25,41 @@ class TestArvPut < Minitest::Test
     assert_match /^usage:/, out
   end
 
+  def test_raw_stdin
+    out, err = capture_subprocess_io do
+      r,w = IO.pipe
+      wpid = fork do
+        r.close
+        w << 'foo'
+      end
+      w.close
+      assert_equal true, arv_put('--raw', {in: r})
+      r.close
+      Process.waitpid wpid
+    end
+    $stderr.write err
+    assert_match '', err
+    assert_equal "acbd18db4cc2f85cedef654fccc4a4d8+3\n", out
+  end
+
+  def test_raw_file
+    out, err = capture_subprocess_io do
+      assert_equal true, arv_put('--raw', './tmp/foo')
+    end
+    $stderr.write err
+    assert_match '', err
+    assert_equal "acbd18db4cc2f85cedef654fccc4a4d8+3\n", out
+  end
+
+  def test_raw_empty_file
+    out, err = capture_subprocess_io do
+      assert_equal true, arv_put('--raw', './tmp/empty_file')
+    end
+    $stderr.write err
+    assert_match '', err
+    assert_equal "d41d8cd98f00b204e9800998ecf8427e+0\n", out
+  end
+
   def test_filename_arg_with_directory
     out, err = capture_subprocess_io do
       assert_equal(false, arv_put('--filename', 'foo', './tmp/empty_dir/.'),
@@ -116,16 +151,48 @@ class TestArvPut < Minitest::Test
     assert_equal foo_manifest_locator+"\n", out
   end
 
+  def test_read_from_implicit_stdin_implicit_manifest
+    test_read_from_stdin_implicit_manifest(specify_stdin_as=nil,
+                                           expect_filename='stdin')
+  end
+
+  def test_read_from_dev_stdin_implicit_manifest
+    test_read_from_stdin_implicit_manifest(specify_stdin_as='/dev/stdin')
+  end
+
+  def test_read_from_stdin_implicit_manifest(specify_stdin_as='-',
+                                             expect_filename=nil)
+    expect_filename = expect_filename || specify_stdin_as.split('/').last
+    out, err = capture_subprocess_io do
+      r,w = IO.pipe
+      wpid = fork do
+        r.close
+        w << 'foo'
+      end
+      w.close
+      args = []
+      args.push specify_stdin_as if specify_stdin_as
+      assert_equal true, arv_put(*args, { in: r })
+      r.close
+      Process.waitpid wpid
+    end
+    $stderr.write err
+    assert_match '', err
+    assert_equal(foo_manifest_locator(expect_filename)+"\n",
+                 out)
+  end
+
   protected
   def arv_put(*args)
     system ['./bin/arv-put', 'arv-put'], *args
   end
 
-  def foo_manifest
-    ". #{Digest::MD5.hexdigest('foo')}+3 0:3:foo\n"
+  def foo_manifest(filename='foo')
+    ". #{Digest::MD5.hexdigest('foo')}+3 0:3:#{filename}\n"
   end
 
-  def foo_manifest_locator
-    Digest::MD5.hexdigest(foo_manifest) + "+#{foo_manifest.length}"
+  def foo_manifest_locator(filename='foo')
+    Digest::MD5.hexdigest(foo_manifest(filename)) +
+      "+#{foo_manifest(filename).length}"
   end
 end
index ce443b3c45b14ca1787fabf4f876d463319aea6d..a47de3063e1696a37691515f95c11bf7fc91e581 100755 (executable)
@@ -44,9 +44,7 @@ group.add_argument('--manifest', action='store_true',
                    help="""
 Store the file data and resulting manifest in Keep, save a Collection
 object in Arvados, and display the manifest locator (Collection uuid)
-on stdout. This is the default behavior if more than one path argument
-is given, or the path given is a directory, or a --filename argument
-is given.
+on stdout. This is the default behavior.
 """)
 group.add_argument('--as-raw', action='store_true', dest='raw',
                    help="""
@@ -55,9 +53,8 @@ Synonym for --raw.
 group.add_argument('--raw', action='store_true',
                    help="""
 Store the file content and display the data block locators on stdout,
-separated by spaces, with a trailing newline. Do not store a
-manifest. This is the default behavior when reading data from a single
-file or standard input.
+separated by commas, with a trailing newline. Do not store a
+manifest.
 """)
 parser.add_argument('--use-filename', type=str, default=None, dest='filename',
                     help="""
@@ -99,10 +96,6 @@ if len(args.paths) != 1 or os.path.isdir(args.paths[0]):
 --filename argument cannot be used when storing a directory or
 multiple files.
 """)
-elif not args.filename and not args.stream and not args.manifest:
-    # When reading from a single non-directory, and no --filename is
-    # given, default to writing raw blocks rather than a manifest.
-    args.raw = True
 
 # Turn on --progress by default if stderr is a tty.
 if (not (args.batch_progress or args.no_progress)
@@ -154,7 +147,10 @@ elif args.batch_progress:
 else:
     writer = arvados.CollectionWriter()
 
-args.paths = [('/dev/stdin' if p=='-' else p) for p in args.paths]
+if args.paths == ['-']:
+    args.paths = ['/dev/stdin']
+    if not args.filename:
+        args.filename = '-'
 
 # Walk the given directory trees and stat files, adding up file sizes,
 # so we can display progress as percent