Merge branch '17840-unparsed-args'
authorTom Clegg <tom@curii.com>
Thu, 18 Nov 2021 16:37:06 +0000 (11:37 -0500)
committerTom Clegg <tom@curii.com>
Thu, 18 Nov 2021 16:37:06 +0000 (11:37 -0500)
fixes #17840

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

12 files changed:
AUTHORS
CONTRIBUTING.md
doc/admin/collection-managed-properties.html.textile.liquid
sdk/cwl/setup.py
sdk/python/arvados/commands/arv_copy.py
sdk/python/setup.py
sdk/python/tests/test_arv_copy.py
services/api/app/controllers/arvados/v1/groups_controller.rb
services/api/test/functional/arvados/v1/groups_controller_test.rb
services/keepstore/command.go
services/keepstore/handlers.go
services/keepstore/unix_volume.go

diff --git a/AUTHORS b/AUTHORS
index 93bdb37dc8f89fa0e8452804beee4943faa9c22e..b8b75518ff08ad0f0d787f27f52ba8f58f2bece2 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -20,4 +20,5 @@ Chen Chen <aflyhorse@gmail.com>
 Veritas Genetics, Inc. <*@veritasgenetics.com>
 Curii Corporation, Inc. <*@curii.com>
 Dante Tsang <dante@dantetsang.com>
-Codex Genetics Ltd <info@codexgenetics.com>
\ No newline at end of file
+Codex Genetics Ltd <info@codexgenetics.com>
+Bruno P. Kinoshita <brunodepaulak@yahoo.com.br>
index 39483ce62d879d5e7c8ba645315b0041f5271bd1..3376311e5c80690cd6088e951d6d9582e504e2d0 100644 (file)
@@ -14,19 +14,19 @@ hear about Arvados success stories.
 Those interested in contributing should begin by joining the [Arvados community
 channel](https://gitter.im/arvados/community) and telling us about your interest.
 
-Contributers should also create an account at https://dev.arvados.org
+Contributors should also create an account at https://dev.arvados.org
 to be able to create and comment on bug tracker issues.  The
 Arvados public bug tracker is located at
 https://dev.arvados.org/projects/arvados/issues .
 
-Contributers may also be interested in the [development road map](https://dev.arvados.org/issues/gantt?utf8=%E2%9C%93&set_filter=1&gantt=1&f%5B%5D=project_id&op%5Bproject_id%5D=%3D&v%5Bproject_id%5D%5B%5D=49&f%5B%5D=&zoom=1).
+Contributors may also be interested in the [development road map](https://dev.arvados.org/issues/gantt?utf8=%E2%9C%93&set_filter=1&gantt=1&f%5B%5D=project_id&op%5Bproject_id%5D=%3D&v%5Bproject_id%5D%5B%5D=49&f%5B%5D=&zoom=1).
 
 # Development
 
 Git repositories for primary development are located at
 https://git.arvados.org/ and can also be browsed at
 https://dev.arvados.org/projects/arvados/repository .  Every push to
-the master branch is also mirrored to Github at
+the main branch is also mirrored to Github at
 https://github.com/arvados/arvados .
 
 Visit [Hacking Arvados](https://dev.arvados.org/projects/arvados/wiki/Hacking) for
@@ -46,12 +46,12 @@ This is the general contribution process:
 2. Clone your fork, make your changes, commit to your fork.
 3. Every commit message must have a DCO sign-off and every file must have a SPDX license (see below).
 4. Add yourself to the [AUTHORS](AUTHORS) file
-5. When your fork is ready, through Github, Create a Pull Request against `arvados:master`
+5. When your fork is ready, through Github, Create a Pull Request against `arvados:main`
 6. Notify the core team about your pull request through the [Arvados development
 channel](https://gitter.im/arvados/development) or by other means.
 7. A member of the core team will review the pull request.  They may have questions or comments, or request changes.
 8. When the contribution is ready, a member of the core team will
-merge the pull request into the master branch, which will
+merge the pull request into the main branch, which will
 automatically resolve the pull request.
 
 The Arvados project does not require a contributor agreement in advance, but does require each commit message include a [Developer Certificate of Origin](https://dev.arvados.org/projects/arvados/wiki/Developer_Certificate_Of_Origin).  Please ensure *every git commit message* includes `Arvados-DCO-1.1-Signed-off-by`. If you have already made commits without it, fix them with `git commit --amend` or `git rebase`.
@@ -68,7 +68,7 @@ New files must also include `SPDX-License-Identifier` at the top with one of the
 
 Continuous integration is hosted at https://ci.arvados.org/
 
-Currently, external contributers cannot trigger builds.  We are investigating integration with Github pull requests for the future.
+Currently, external contributors cannot trigger builds.  We are investigating integration with Github pull requests for the future.
 
 [![Build Status](https://ci.arvados.org/buildStatus/icon?job=run-tests)](https://ci.arvados.org/job/run-tests/)
 
index 39520012639d9607b4314513378c4f0b45b061e7..341030c418d1e88030a36f6b7e422f760e5940f6 100644 (file)
@@ -41,13 +41,23 @@ h4. Protected properties
 
 If there's a need to prevent a non-admin user from modifying a specific property, even by its owner, the @Protected@ attribute can be set to @true@, like so:
 
+<pre>
+Collections:
+  ManagedProperties:
+    sample_id: {Protected: true}
+</pre>
+
+This configuration won't assign a @sample_id@ property on collection creation, but if the user adds it to any collection, its value is protected from that point on.
+
+Another use case would be to protect properties that were automatically assigned by the system:
+
 <pre>
 Collections:
   ManagedProperties:
     responsible_person_uuid: {Function: original_owner, Protected: true}
 </pre>
 
-This property can be applied to any of the defined managed properties. If missing, it's assumed as being @false@ by default.
+If missing, the @Protected@ attribute it’s assumed as being @false@ by default.
 
 h3. Supporting example scripts
 
index e39fdd8d9f2db8259a210649d279b08f163d054e..f034ca5ab2f1ed82a385431fb1e32acea7c17fc7 100644 (file)
@@ -39,8 +39,8 @@ setup(name='arvados-cwl-runner',
       # file to determine what version of cwltool and schema-salad to
       # build.
       install_requires=[
-          'cwltool==3.1.20211020155521',
-          'schema-salad==8.2.20211020114435',
+          'cwltool==3.1.20211107152837',
+          'schema-salad==8.2.20211116214159',
           'arvados-python-client{}'.format(pysdk_dep),
           'setuptools',
           'ciso8601 >= 2.0.0',
index d10234ca601f907f9d429efd749e1af03a85d492..7951842acc6741c52b2669400f7082171c68d377 100755 (executable)
@@ -113,7 +113,7 @@ def main():
     copy_opts.set_defaults(recursive=True)
 
     parser = argparse.ArgumentParser(
-        description='Copy a workflow, collection or project from one Arvados instance to another.',
+        description='Copy a workflow, collection or project from one Arvados instance to another.  On success, the uuid of the copied object is printed to stdout.',
         parents=[copy_opts, arv_cmd.retry_opt])
     args = parser.parse_args()
 
@@ -161,7 +161,12 @@ def main():
         logger.error("API server returned an error result: {}".format(result))
         exit(1)
 
-    logger.info("")
+    print(result['uuid'])
+
+    if result.get('partial_error'):
+        logger.warning("Warning: created copy with uuid {} but failed to copy some items: {}".format(result['uuid'], result['partial_error']))
+        exit(1)
+
     logger.info("Success: created copy with uuid {}".format(result['uuid']))
     exit(0)
 
@@ -292,8 +297,11 @@ def copy_workflow(wf_uuid, src, dst, args):
     # fetch the workflow from the source instance
     wf = src.workflows().get(uuid=wf_uuid).execute(num_retries=args.retries)
 
+    if not wf["definition"]:
+        logger.warning("Workflow object {} has an empty or null definition, it won't do anything.".format(wf_uuid))
+
     # copy collections and docker images
-    if args.recursive:
+    if args.recursive and wf["definition"]:
         wf_def = yaml.safe_load(wf["definition"])
         if wf_def is not None:
             locations = []
@@ -683,17 +691,31 @@ def copy_project(obj_uuid, src, dst, owner_uuid, args):
 
     logger.debug('Copying %s to %s', obj_uuid, project_record["uuid"])
 
+
+    partial_error = ""
+
     # Copy collections
-    copy_collections([col["uuid"] for col in arvados.util.list_all(src.collections().list, filters=[["owner_uuid", "=", obj_uuid]])],
-                     src, dst, args)
+    try:
+        copy_collections([col["uuid"] for col in arvados.util.list_all(src.collections().list, filters=[["owner_uuid", "=", obj_uuid]])],
+                         src, dst, args)
+    except Exception as e:
+        partial_error += "\n" + str(e)
 
     # Copy workflows
     for w in arvados.util.list_all(src.workflows().list, filters=[["owner_uuid", "=", obj_uuid]]):
-        copy_workflow(w["uuid"], src, dst, args)
+        try:
+            copy_workflow(w["uuid"], src, dst, args)
+        except Exception as e:
+            partial_error += "\n" + "Error while copying %s: %s" % (w["uuid"], e)
 
     if args.recursive:
         for g in arvados.util.list_all(src.groups().list, filters=[["owner_uuid", "=", obj_uuid]]):
-            copy_project(g["uuid"], src, dst, project_record["uuid"], args)
+            try:
+                copy_project(g["uuid"], src, dst, project_record["uuid"], args)
+            except Exception as e:
+                partial_error += "\n" + "Error while copying %s: %s" % (g["uuid"], e)
+
+    project_record["partial_error"] = partial_error
 
     return project_record
 
index 8d637303b466f90712b9ef4b44846277d5af6ed0..f82d44ab6033072915044e0514663ad9e1890025 100644 (file)
@@ -50,7 +50,7 @@ setup(name='arvados-python-client',
           'future',
           'google-api-python-client >=1.6.2, <2',
           'google-auth<2',
-          'httplib2 >=0.9.2',
+          'httplib2 >=0.9.2, <0.20.2',
           'pycurl >=7.19.5.1',
           'ruamel.yaml >=0.15.54, <0.17.11',
           'setuptools',
index b560018d385bfd3f62f366333527f54a3ec272c2..b853b330435ac758a2a4f3c25bc02cfb5c89a12a 100644 (file)
@@ -61,10 +61,13 @@ class ArvCopyVersionTestCase(run_test_server.TestCaseWithServers, tutil.VersionC
             contents = api.groups().list(filters=[["owner_uuid", "=", dest_proj]]).execute()
             assert len(contents["items"]) == 0
 
-            try:
-                self.run_copy(["--project-uuid", dest_proj, "--storage-classes", "foo", src_proj])
-            except SystemExit as e:
-                assert e.code == 0
+            with tutil.redirected_streams(
+                    stdout=tutil.StringIO, stderr=tutil.StringIO) as (out, err):
+                try:
+                    self.run_copy(["--project-uuid", dest_proj, "--storage-classes", "foo", src_proj])
+                except SystemExit as e:
+                    assert e.code == 0
+                copy_uuid_from_stdout = out.getvalue().strip()
 
             contents = api.groups().list(filters=[["owner_uuid", "=", dest_proj]]).execute()
             assert len(contents["items"]) == 1
@@ -72,6 +75,8 @@ class ArvCopyVersionTestCase(run_test_server.TestCaseWithServers, tutil.VersionC
             assert contents["items"][0]["name"] == "arv-copy project"
             copied_project = contents["items"][0]["uuid"]
 
+            assert copied_project == copy_uuid_from_stdout
+
             contents = api.collections().list(filters=[["owner_uuid", "=", copied_project]]).execute()
             assert len(contents["items"]) == 1
 
index 8d15bb1c5062a0215a04f60fb9749cd7ad7e35d1..7fbb86c010c01df4d08cbde40c434c7f8b9063ae 100644 (file)
@@ -10,6 +10,8 @@ class Arvados::V1::GroupsController < ApplicationController
   skip_before_action :find_object_by_uuid, only: :shared
   skip_before_action :render_404_if_no_object, only: :shared
 
+  TRASHABLE_CLASSES = ['project']
+
   def self._index_requires_parameters
     (super rescue {}).
       merge({
@@ -99,6 +101,15 @@ class Arvados::V1::GroupsController < ApplicationController
     end
   end
 
+  def destroy
+    if !TRASHABLE_CLASSES.include?(@object.group_class)
+      return @object.destroy
+      show
+    else
+      super # Calls destroy from TrashableController module
+    end
+  end
+
   def render_404_if_no_object
     if params[:action] == 'contents'
       if !params[:uuid]
@@ -351,8 +362,6 @@ class Arvados::V1::GroupsController < ApplicationController
     @offset = offset_all
   end
 
-  protected
-
   def exclude_home objectlist, klass
     # select records that are readable by current user AND
     #   the owner_uuid is a user (but not the current user) OR
index 02a4ce96632d2962b830a720fbe3621458e79fb8..4dbccc5eb24edd07cdb09e6f2e400faa9c4f0c81 100644 (file)
@@ -538,6 +538,21 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
     assert_includes(owners, groups(:asubproject).uuid)
   end
 
+  [:afiltergroup, :private_role].each do |grp|
+    test "delete non-project group #{grp}" do
+      authorize_with :admin
+      assert_not_nil Group.find_by_uuid(groups(grp).uuid)
+      assert !Group.find_by_uuid(groups(grp).uuid).is_trashed
+      post :destroy, params: {
+            id: groups(grp).uuid,
+            format: :json,
+          }
+      assert_response :success
+      # Should not be trashed
+      assert_nil Group.find_by_uuid(groups(grp).uuid)
+    end
+  end
+
   ### trashed project tests ###
 
   #
index 94dabfda6207d564fc605cce499c967f0af25b35..555f16dfe1f290edbd1797437efd3842ad29dd4e 100644 (file)
@@ -27,7 +27,6 @@ import (
 )
 
 var (
-       version = "dev"
        Command = service.Command(arvados.ServiceNameKeepstore, newHandlerOrErrorHandler)
 )
 
@@ -167,7 +166,7 @@ func (h *handler) setup(ctx context.Context, cluster *arvados.Cluster, token str
                return errors.New("no volumes configured")
        }
 
-       h.Logger.Printf("keepstore %s starting, pid %d", version, os.Getpid())
+       h.Logger.Printf("keepstore %s starting, pid %d", cmd.Version.String(), os.Getpid())
 
        // Start a round-robin VolumeManager with the configured volumes.
        vm, err := makeRRVolumeManager(h.Logger, h.Cluster, serviceURL, newVolumeMetricsVecs(reg))
index 2a90705a56cbf0c1eec78a03dcd5b45463f84851..52683d97162c448c82a694641943f1fad07063c5 100644 (file)
@@ -21,6 +21,7 @@ import (
        "sync/atomic"
        "time"
 
+       "git.arvados.org/arvados.git/lib/cmd"
        "git.arvados.org/arvados.git/sdk/go/arvados"
        "git.arvados.org/arvados.git/sdk/go/ctxlog"
        "git.arvados.org/arvados.git/sdk/go/health"
@@ -390,7 +391,7 @@ func (rtr *router) StatusHandler(resp http.ResponseWriter, req *http.Request) {
 
 // populate the given NodeStatus struct with current values.
 func (rtr *router) readNodeStatus(st *NodeStatus) {
-       st.Version = version
+       st.Version = strings.SplitN(cmd.Version.String(), " ", 2)[0]
        vols := rtr.volmgr.AllReadable()
        if cap(st.Volumes) < len(vols) {
                st.Volumes = make([]*volumeStatusEnt, len(vols))
index f076ccf18419675499e12eed0e3d017824af8e57..46f4db4095bfb286c82f4f07a988c16cee5ebe63 100644 (file)
@@ -359,47 +359,53 @@ var blockFileRe = regexp.MustCompile(`^[0-9a-f]{32}$`)
 //     e4de7a2810f5554cd39b36d8ddb132ff+67108864 1388701136
 //
 func (v *UnixVolume) IndexTo(prefix string, w io.Writer) error {
-       var lastErr error
        rootdir, err := v.os.Open(v.Root)
        if err != nil {
                return err
        }
-       defer rootdir.Close()
        v.os.stats.TickOps("readdir")
        v.os.stats.Tick(&v.os.stats.ReaddirOps)
-       for {
-               names, err := rootdir.Readdirnames(1)
-               if err == io.EOF {
-                       return lastErr
-               } else if err != nil {
-                       return err
-               }
-               if !strings.HasPrefix(names[0], prefix) && !strings.HasPrefix(prefix, names[0]) {
+       subdirs, err := rootdir.Readdirnames(-1)
+       rootdir.Close()
+       if err != nil {
+               return err
+       }
+       for _, subdir := range subdirs {
+               if !strings.HasPrefix(subdir, prefix) && !strings.HasPrefix(prefix, subdir) {
                        // prefix excludes all blocks stored in this dir
                        continue
                }
-               if !blockDirRe.MatchString(names[0]) {
+               if !blockDirRe.MatchString(subdir) {
                        continue
                }
-               blockdirpath := filepath.Join(v.Root, names[0])
+               blockdirpath := filepath.Join(v.Root, subdir)
                blockdir, err := v.os.Open(blockdirpath)
                if err != nil {
                        v.logger.WithError(err).Errorf("error reading %q", blockdirpath)
-                       lastErr = fmt.Errorf("error reading %q: %s", blockdirpath, err)
-                       continue
+                       return fmt.Errorf("error reading %q: %s", blockdirpath, err)
                }
                v.os.stats.TickOps("readdir")
                v.os.stats.Tick(&v.os.stats.ReaddirOps)
-               for {
-                       fileInfo, err := blockdir.Readdir(1)
-                       if err == io.EOF {
-                               break
+               // ReadDir() (compared to Readdir(), which returns
+               // FileInfo structs) helps complete the sequence of
+               // readdirent calls as quickly as possible, reducing
+               // the likelihood of NFS EBADCOOKIE (523) errors.
+               dirents, err := blockdir.ReadDir(-1)
+               blockdir.Close()
+               if err != nil {
+                       v.logger.WithError(err).Errorf("error reading %q", blockdirpath)
+                       return fmt.Errorf("error reading %q: %s", blockdirpath, err)
+               }
+               for _, dirent := range dirents {
+                       fileInfo, err := dirent.Info()
+                       if os.IsNotExist(err) {
+                               // File disappeared between ReadDir() and now
+                               continue
                        } else if err != nil {
-                               v.logger.WithError(err).Errorf("error reading %q", blockdirpath)
-                               lastErr = fmt.Errorf("error reading %q: %s", blockdirpath, err)
-                               break
+                               v.logger.WithError(err).Errorf("error getting FileInfo for %q in %q", dirent.Name(), blockdirpath)
+                               return err
                        }
-                       name := fileInfo[0].Name()
+                       name := fileInfo.Name()
                        if !strings.HasPrefix(name, prefix) {
                                continue
                        }
@@ -408,16 +414,15 @@ func (v *UnixVolume) IndexTo(prefix string, w io.Writer) error {
                        }
                        _, err = fmt.Fprint(w,
                                name,
-                               "+", fileInfo[0].Size(),
-                               " ", fileInfo[0].ModTime().UnixNano(),
+                               "+", fileInfo.Size(),
+                               " ", fileInfo.ModTime().UnixNano(),
                                "\n")
                        if err != nil {
-                               blockdir.Close()
                                return fmt.Errorf("error writing: %s", err)
                        }
                }
-               blockdir.Close()
        }
+       return nil
 }
 
 // Trash trashes the block data from the unix storage