[ARVADOS] updated: 1.3.0-2383-gf985b2fac

Git user git at public.arvados.org
Tue Mar 31 02:14:46 UTC 2020


Summary of changes:
 lib/controller/federation/conn.go                  | 116 ++++++++++++---------
 lib/controller/federation/list.go                  |   7 ++
 sdk/go/arvados/api.go                              |   2 +-
 sdk/python/arvados/commands/federation_migrate.py  |  22 ++--
 .../api/app/controllers/application_controller.rb  |   5 +
 .../app/controllers/arvados/v1/users_controller.rb |   6 --
 6 files changed, 92 insertions(+), 66 deletions(-)

       via  f985b2fac7d90d9a54d5a115f8c067ff795a018c (commit)
      from  fecdfe882983e1cab45b204a4876ffe21e36f542 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit f985b2fac7d90d9a54d5a115f8c067ff795a018c
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Mon Mar 30 22:14:23 2020 -0400

    16263: Generalize "local_user_list" flag to "no_federation"
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 519ea21b9..f67ea6713 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -365,64 +365,76 @@ var userAttrsCachedFromLoginCluster = map[string]bool{
 	"writable_by":             false,
 }
 
-func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
+func (conn *Conn) batchUpdateUsers(ctx context.Context,
+	options arvados.ListOptions,
+	items []arvados.User) (err error) {
+
+	id := conn.cluster.Login.LoginCluster
 	logger := ctxlog.FromContext(ctx)
-	if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.LocalUserList {
-		resp, err := conn.chooseBackend(id).UserList(ctx, options)
-		if err != nil {
-			return resp, err
+	batchOpts := arvados.UserBatchUpdateOptions{Updates: map[string]map[string]interface{}{}}
+	for _, user := range items {
+		if !strings.HasPrefix(user.UUID, id) {
+			continue
+		}
+		logger.Debugf("cache user info for uuid %q", user.UUID)
+
+		// If the remote cluster has null timestamps
+		// (e.g., test server with incomplete
+		// fixtures) use dummy timestamps (instead of
+		// the zero time, which causes a Rails API
+		// error "year too big to marshal: 1 UTC").
+		if user.ModifiedAt.IsZero() {
+			user.ModifiedAt = time.Now()
+		}
+		if user.CreatedAt.IsZero() {
+			user.CreatedAt = time.Now()
 		}
-		batchOpts := arvados.UserBatchUpdateOptions{Updates: map[string]map[string]interface{}{}}
-		for _, user := range resp.Items {
-			if !strings.HasPrefix(user.UUID, id) {
-				continue
-			}
-			logger.Debugf("cache user info for uuid %q", user.UUID)
-
-			// If the remote cluster has null timestamps
-			// (e.g., test server with incomplete
-			// fixtures) use dummy timestamps (instead of
-			// the zero time, which causes a Rails API
-			// error "year too big to marshal: 1 UTC").
-			if user.ModifiedAt.IsZero() {
-				user.ModifiedAt = time.Now()
-			}
-			if user.CreatedAt.IsZero() {
-				user.CreatedAt = time.Now()
-			}
 
-			var allFields map[string]interface{}
-			buf, err := json.Marshal(user)
-			if err != nil {
-				return arvados.UserList{}, fmt.Errorf("error encoding user record from remote response: %s", err)
-			}
-			err = json.Unmarshal(buf, &allFields)
-			if err != nil {
-				return arvados.UserList{}, fmt.Errorf("error transcoding user record from remote response: %s", err)
-			}
-			updates := allFields
-			if len(options.Select) > 0 {
-				updates = map[string]interface{}{}
-				for _, k := range options.Select {
-					if v, ok := allFields[k]; ok && userAttrsCachedFromLoginCluster[k] {
-						updates[k] = v
-					}
+		var allFields map[string]interface{}
+		buf, err := json.Marshal(user)
+		if err != nil {
+			return fmt.Errorf("error encoding user record from remote response: %s", err)
+		}
+		err = json.Unmarshal(buf, &allFields)
+		if err != nil {
+			return fmt.Errorf("error transcoding user record from remote response: %s", err)
+		}
+		updates := allFields
+		if len(options.Select) > 0 {
+			updates = map[string]interface{}{}
+			for _, k := range options.Select {
+				if v, ok := allFields[k]; ok && userAttrsCachedFromLoginCluster[k] {
+					updates[k] = v
 				}
-			} else {
-				for k := range updates {
-					if !userAttrsCachedFromLoginCluster[k] {
-						delete(updates, k)
-					}
+			}
+		} else {
+			for k := range updates {
+				if !userAttrsCachedFromLoginCluster[k] {
+					delete(updates, k)
 				}
 			}
-			batchOpts.Updates[user.UUID] = updates
 		}
-		if len(batchOpts.Updates) > 0 {
-			ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
-			_, err = conn.local.UserBatchUpdate(ctxRoot, batchOpts)
-			if err != nil {
-				return arvados.UserList{}, fmt.Errorf("error updating local user records: %s", err)
-			}
+		batchOpts.Updates[user.UUID] = updates
+	}
+	if len(batchOpts.Updates) > 0 {
+		ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
+		_, err = conn.local.UserBatchUpdate(ctxRoot, batchOpts)
+		if err != nil {
+			return fmt.Errorf("error updating local user records: %s", err)
+		}
+	}
+	return nil
+}
+
+func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
+	if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.NoFederation {
+		resp, err := conn.chooseBackend(id).UserList(ctx, options)
+		if err != nil {
+			return resp, err
+		}
+		err = conn.batchUpdateUsers(ctx, options, resp.Items)
+		if err != nil {
+			return arvados.UserList{}, err
 		}
 		return resp, nil
 	} else {
@@ -439,7 +451,7 @@ func (conn *Conn) UserUpdate(ctx context.Context, options arvados.UpdateOptions)
 }
 
 func (conn *Conn) UserUpdateUUID(ctx context.Context, options arvados.UpdateUUIDOptions) (arvados.User, error) {
-	return conn.chooseBackend(options.UUID).UserUpdateUUID(ctx, options)
+	return conn.local.UserUpdateUUID(ctx, options)
 }
 
 func (conn *Conn) UserMerge(ctx context.Context, options arvados.UserMergeOptions) (arvados.User, error) {
diff --git a/lib/controller/federation/list.go b/lib/controller/federation/list.go
index 6ee813317..d1d52cfc6 100644
--- a/lib/controller/federation/list.go
+++ b/lib/controller/federation/list.go
@@ -106,6 +106,13 @@ func (conn *Conn) generated_CollectionList(ctx context.Context, options arvados.
 // corresponding options argument suitable for sending to that
 // backend.
 func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions, fn func(context.Context, string, arvados.API, arvados.ListOptions) ([]string, error)) error {
+
+	if opts.NoFederation {
+		// Client requested no federation.  Pass through.
+		_, err := fn(ctx, conn.cluster.ClusterID, conn.local, opts)
+		return err
+	}
+
 	cannotSplit := false
 	var matchAllFilters map[string]bool
 	for _, f := range opts.Filters {
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 3b20955a1..a30da6242 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -84,7 +84,7 @@ type ListOptions struct {
 	Count              string                 `json:"count"`
 	IncludeTrash       bool                   `json:"include_trash"`
 	IncludeOldVersions bool                   `json:"include_old_versions"`
-	LocalUserList      bool                   `json:"local_user_list"`
+	NoFederation       bool                   `json:"no_federation"`
 }
 
 type CreateOptions struct {
diff --git a/sdk/python/arvados/commands/federation_migrate.py b/sdk/python/arvados/commands/federation_migrate.py
index 344390b48..e1b8ee7d8 100755
--- a/sdk/python/arvados/commands/federation_migrate.py
+++ b/sdk/python/arvados/commands/federation_migrate.py
@@ -98,7 +98,7 @@ def fetch_users(clusters, loginCluster):
     users = []
     for c, arv in clusters.items():
         print("Getting user list from %s" % c)
-        ul = arvados.util.list_all(arv.users().list, local_user_list=True)
+        ul = arvados.util.list_all(arv.users().list, no_federation=True)
         for l in ul:
             if l["uuid"].startswith(c):
                 users.append(l)
@@ -171,7 +171,7 @@ def update_username(args, email, user_uuid, username, migratecluster, migratearv
     print("(%s) Updating username of %s to '%s' on %s" % (email, user_uuid, username, migratecluster))
     if not args.dry_run:
         try:
-            conflicts = migratearv.users().list(filters=[["username", "=", username]], local_user_list=True).execute()
+            conflicts = migratearv.users().list(filters=[["username", "=", username]], no_federation=True).execute()
             if conflicts["items"]:
                 migratearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute()
             migratearv.users().update(uuid=user_uuid, body={"user": {"username": username}}).execute()
@@ -204,7 +204,7 @@ def choose_new_user(args, by_email, email, userhome, username, old_user_uuid, cl
             user = None
             try:
                 olduser = oldhomearv.users().get(uuid=old_user_uuid).execute()
-                conflicts = homearv.users().list(filters=[["username", "=", username]], local_user_list=True).execute()
+                conflicts = homearv.users().list(filters=[["username", "=", username]], no_federation=True).execute()
                 if conflicts["items"]:
                     homearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute()
                 user = homearv.users().create(body={"user": {"email": email, "username": username, "is_active": olduser["is_active"]}}).execute()
@@ -241,10 +241,16 @@ def activate_remote_user(args, email, homearv, migratearv, old_user_uuid, new_us
         return None
 
     try:
-        olduser = migratearv.users().get(uuid=old_user_uuid).execute()
+        findolduser = migratearv.users().list(filters=[["uuid", "=", old_user_uuid]], no_federation=True).execute()
+        if len(findolduser["items"]) == 0:
+            return False
+        if len(findolduser["items"]) == 1:
+            olduser = findolduser["items"][0]
+        else:
+            print("(%s) Unexpected result" % (email))
+            return None
     except arvados.errors.ApiError as e:
-        if e.resp.status != 404:
-            print("(%s) Could not retrieve user %s from %s, user may have already been migrated: %s" % (email, old_user_uuid, migratecluster, e))
+        print("(%s) Could not retrieve user %s from %s, user may have already been migrated: %s" % (email, old_user_uuid, migratecluster, e))
         return None
 
     salted = 'v2/' + newtok["uuid"] + '/' + hmac.new(newtok["api_token"].encode(),
@@ -351,10 +357,10 @@ def main():
             if new_user_uuid is None:
                 continue
 
-            # cluster where the migration is happening
             remote_users = {}
             got_error = False
             for migratecluster in clusters:
+                # cluster where the migration is happening
                 migratearv = clusters[migratecluster]
 
                 # the user's new home cluster
@@ -370,6 +376,8 @@ def main():
                 for migratecluster in clusters:
                     migratearv = clusters[migratecluster]
                     newuser = remote_users[migratecluster]
+                    if newuser is False:
+                        continue
 
                     print("(%s) Migrating %s to %s on %s" % (email, old_user_uuid, new_user_uuid, migratecluster))
 
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 7b82cdb61..68fa7d880 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -656,6 +656,11 @@ class ApplicationController < ActionController::Base
         location: "query",
         required: false,
       },
+      no_federation: {
+        type: 'boolean',
+        required: false,
+        description: 'bypass federation behavior, list items from local instance database only'
+      }
     }
   end
 
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 9439493df..289e82567 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -257,12 +257,6 @@ class Arvados::V1::UsersController < ApplicationController
     }
   end
 
-  def self._index_requires_parameters
-    super.merge(
-      { local_user_list: {required: false, type: 'boolean',
-                          description: 'only list users from local database, no effect if LoginCluster is not set'} })
-  end
-
   def apply_filters(model_class=nil)
     return super if @read_users.any?(&:is_admin)
     if params[:uuid] != current_user.andand.uuid

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list