[ARVADOS] updated: ed6af9cb44515fe1759eeebabfcac4a068fd697c

Git user git at public.curoverse.com
Wed Oct 18 13:27:24 EDT 2017


Summary of changes:
 sdk/cli/bin/arv-sync-groups                |   7 -
 sdk/python/arvados/commands/sync_groups.py | 190 ----------------
 tools/arv-sync-groups/arv-sync-groups.go   | 341 +++++++++++++++++++----------
 3 files changed, 221 insertions(+), 317 deletions(-)
 delete mode 100755 sdk/cli/bin/arv-sync-groups
 delete mode 100644 sdk/python/arvados/commands/sync_groups.py

       via  ed6af9cb44515fe1759eeebabfcac4a068fd697c (commit)
      from  1cde975fed7a57b1397bded4d73502bf4b98f517 (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 ed6af9cb44515fe1759eeebabfcac4a068fd697c
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Wed Oct 18 14:17:01 2017 -0300

    12018: Several changes, listed below:
    * Removed initial Python version.
    * Removed superfluous input files checks and moved file opening
    to be before any API calls, to avoid doing them is there's a problem
    with the file.
    * Added user, group & link types so they're populated by json decoding.
    * Cleaned up ListAll() func so it can work with different resource
    types.
    * Changed usage of set style types from map[string]struct{} to be
    map[string]bool to simplify membership checking.
    * Corrected error messages to start with lowercase.
    * Added more debug messages for -verbose mode.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/sdk/cli/bin/arv-sync-groups b/sdk/cli/bin/arv-sync-groups
deleted file mode 100755
index f744bff..0000000
--- a/sdk/cli/bin/arv-sync-groups
+++ /dev/null
@@ -1,7 +0,0 @@
-#!/usr/bin/env python
-# Copyright (C) The Arvados Authors. All rights reserved.
-#
-# SPDX-License-Identifier: Apache-2.0
-
-from arvados.commands.sync_groups import main
-main()
diff --git a/sdk/python/arvados/commands/sync_groups.py b/sdk/python/arvados/commands/sync_groups.py
deleted file mode 100644
index a1125de..0000000
--- a/sdk/python/arvados/commands/sync_groups.py
+++ /dev/null
@@ -1,190 +0,0 @@
-# Copyright (C) The Arvados Authors. All rights reserved.
-#
-# SPDX-License-Identifier: Apache-2.0
-
-import argparse
-import arvados
-import csv
-import logging
-import os
-import sys
-
-from apiclient import errors as apiclient_errors
-from arvados._version import __version__
-
-import arvados.commands._util as arv_cmd
-
-api_client = None
-
-GROUP_TAG = 'remote_group'
-
-opts = argparse.ArgumentParser(add_help=False)
-
-opts.add_argument('--version', action='version',
-                    version="%s %s" % (sys.argv[0], __version__),
-                    help='Print version and exit.')
-opts.add_argument('--verbose', action='store_true', default=False,
-                  help="""
-Log informational messages. By default is deactivated.
-""")
-opts.add_argument('path', metavar='PATH', type=str, 
-                    help="""
-Local file path containing a CSV-like format.
-""")
-
-_user_id = opts.add_mutually_exclusive_group()
-_user_id.add_argument('--user-email', action='store_true', default=True,
-                       help="""
-Identify users by their email addresses instead of user names.
-This is the default.
-""")
-_user_id.add_argument('--user-name', action='store_false', dest='user_email',
-                      help="""
-Identify users by their name instead of email addresses.
-""")
-
-arg_parser = argparse.ArgumentParser(
-    description='Synchronize group memberships from a CSV file.',
-    parents=[opts, arv_cmd.retry_opt])
-
-def parse_arguments(arguments):
-    args = arg_parser.parse_args(arguments)
-    if args.path is None or args.path == '':
-        arg_parser.error("Please provide a path to an input file.")
-    elif not os.path.exists(args.path):
-        arg_parser.error("File not found: '%s'" % args.path)
-    elif not os.path.isfile(args.path):
-        arg_parser.error("Path provided is not a file: '%s'" % args.path)
-    return args
-
-def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
-    global api_client
-
-    args = parse_arguments(arguments)
-    logger = logging.getLogger('arvados.arv_sync_groups')
-
-    if api_client is None:
-        api_client = arvados.api('v1')
-
-    # How are users going to be identified on the input file?
-    if args.user_email:
-        user_id = 'email'
-    else:
-        user_id = 'username'
-    
-    if args.verbose:
-        logger.setLevel(logging.INFO)
-        
-    logger.info("Group sync starting. Using '%s' as users id" % user_id)
-    
-    # Get the complete user list to minimize API Server requests
-    all_users = {}
-    userid_to_uuid = {} # Index by user_id (email/username)
-    for u in arvados.util.list_all(api_client.users().list, args.retries):
-        all_users[u['uuid']] = u
-        userid_to_uuid[u[user_id]] = u['uuid']
-    logger.info('Found %d users' % len(all_users))
-
-    # Request all UUIDs for groups tagged as remote
-    remote_group_uuids = set()
-    for link in arvados.util.list_all(
-                            api_client.links().list, 
-                            args.retries,
-                            filters=[['link_class', '=', 'tag'],
-                                     ['name', '=', GROUP_TAG],
-                                     ['head_kind', '=', 'arvados#group']]):
-        remote_group_uuids.add(link['head_uuid'])
-    # Get remote groups and their members
-    remote_groups = {}
-    group_name_to_uuid = {} # Index by group name
-    for group in arvados.util.list_all(
-                            api_client.groups().list,
-                            args.retries,
-                            filters=[['uuid', 'in', list(remote_group_uuids)]]):
-        member_links = arvados.util.list_all(
-                            api_client.links().list,
-                            args.retries,
-                            filters=[['link_class', '=', 'permission'],
-                                      ['name', '=', 'can_read'],
-                                      ['tail_uuid', '=', group['uuid']],
-                                      ['head_kind', '=', 'arvados#user']])
-        # Build a list of user_ids (email/username) belonging to this group
-        members = set([all_users[link['head_uuid']][user_id] 
-                       for link in member_links])
-        remote_groups[group['uuid']] = {'object': group,
-                                        'previous_members': members,
-                                        'current_members': set()}
-        # FIXME: There's an index (group_name, group.owner_uuid), should we
-        # ask for our own groups tagged as remote? (with own being 'system'?)
-        group_name_to_uuid[group['name']] = group['uuid']
-    logger.info('Found %d remote groups' % len(remote_groups))
-    
-    groups_created = 0
-    members_added = 0
-    members_removed = 0
-    with open(args.path, 'rb') as f:
-        reader = csv.reader(f)
-        try:
-            for group, user in reader:
-                group = group.strip()
-                user = user.strip()
-                if not user in userid_to_uuid:
-                    # User not present on the system, skip.
-                    logger.warning("There's no user with %s '%s' on the system"
-                                   ", skipping." % (user_id, user))
-                    continue
-                if not group in group_name_to_uuid:
-                    # Group doesn't exist, create and tag it before continuing
-                    g = api_client.groups().create(body={
-                        'name': group}).execute(num_retries=args.retries)
-                    api_client.links().create(body={
-                        'link_class': 'tag',
-                        'name': GROUP_TAG,
-                        'head_uuid': g['uuid'],
-                    }).execute(num_retries=args.retries)
-                    # Update cached group data
-                    group_name_to_uuid[g['name']] = g['uuid']
-                    remote_groups[g['uuid']] = {'object': g,
-                                                'previous_members': set(),
-                                                'current_members': set()}
-                    groups_created += 1
-                # Both group & user exist, check if user is a member
-                g_uuid = group_name_to_uuid[group]
-                if not (user in remote_groups[g_uuid]['previous_members'] or
-                        user in remote_groups[g_uuid]['current_members']):
-                    # User wasn't a member, but should.
-                    api_client.links().create(body={
-                        'link_class': 'permission',
-                        'name': 'can_read',
-                        'tail_uuid': g_uuid,
-                        'head_uuid': userid_to_uuid[user],
-                    }).execute(num_retries=args.retries)
-                    members_added += 1
-                remote_groups[g_uuid]['current_members'].add(user)
-        except (ValueError, csv.Error) as e:
-            logger.warning('Error on line %d: %s' % (reader.line_num, e))
-    # Remove previous members not listed on this run
-    for group_uuid in remote_groups:
-        previous = remote_groups[group_uuid]['previous_members']
-        current = remote_groups[group_uuid]['current_members']
-        evicted = previous - current
-        if len(evicted) > 0:
-            logger.info("Removing %d users from group '%s'" % (
-                len(evicted), remote_groups[group_uuid]['object']['name']))
-        for evicted_user in evicted:
-            links = arvados.util.list_all(
-                api_client.links().list,
-                args.retries,
-                filters=[['link_class', '=', 'permission'],
-                         ['name', '=', 'can_read'],
-                         ['tail_uuid', '=', group_uuid],
-                         ['head_uuid', '=', userid_to_uuid[evicted_user]]])
-            for l in links:
-                api_client.links().delete(
-                    uuid=l['uuid']).execute(num_retries=args.retries)
-            members_removed += 1
-    logger.info("Groups created: %d, members added: %s, members removed: %d" % \
-                (groups_created, members_added, members_removed))
-
-if __name__ == '__main__':
-    main()
diff --git a/tools/arv-sync-groups/arv-sync-groups.go b/tools/arv-sync-groups/arv-sync-groups.go
index c600c6e..d8af3b0 100644
--- a/tools/arv-sync-groups/arv-sync-groups.go
+++ b/tools/arv-sync-groups/arv-sync-groups.go
@@ -16,6 +16,119 @@ import (
 	"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
 )
 
+type resourceList interface {
+	items() []interface{}
+	itemsAvailable() int
+	offset() int
+}
+
+type groupInfo struct {
+	Group           group
+	PreviousMembers map[string]bool
+	CurrentMembers  map[string]bool
+}
+
+type user struct {
+	UUID     string `json:"uuid,omitempty"`
+	Email    string `json:"email,omitempty"`
+	Username string `json:"username,omitempty"`
+}
+
+func (u user) GetID(idSelector string) (string, error) {
+	switch idSelector {
+	case "email":
+		return u.Email, nil
+	case "username":
+		return u.Username, nil
+	default:
+		return "", fmt.Errorf("cannot identify user by %q selector", idSelector)
+	}
+}
+
+// userList implements resourceList interface
+type userList struct {
+	Items          []user `json:"items"`
+	ItemsAvailable int    `json:"items_available"`
+	Offset         int    `json:"offset"`
+}
+
+func (l userList) items() []interface{} {
+	var out []interface{}
+	for _, item := range l.Items {
+		out = append(out, item)
+	}
+	return out
+}
+
+func (l userList) itemsAvailable() int {
+	return l.ItemsAvailable
+}
+
+func (l userList) offset() int {
+	return l.Offset
+}
+
+type group struct {
+	UUID string `json:"uuid,omitempty"`
+	Name string `json:"name,omitempty"`
+}
+
+// groupList implements resourceList interface
+type groupList struct {
+	Items          []group `json:"items"`
+	ItemsAvailable int     `json:"items_available"`
+	Offset         int     `json:"offset"`
+}
+
+func (l groupList) items() []interface{} {
+	var out []interface{}
+	for _, item := range l.Items {
+		out = append(out, item)
+	}
+	return out
+}
+
+func (l groupList) itemsAvailable() int {
+	return l.ItemsAvailable
+}
+
+func (l groupList) offset() int {
+	return l.Offset
+}
+
+type link struct {
+	UUID      string `json:"uuid, omiempty"`
+	Name      string `json:"name,omitempty"`
+	LinkClass string `json:"link_class,omitempty"`
+	HeadUUID  string `json:"head_uuid,omitempty"`
+	HeadKind  string `json:"head_kind,omitempty"`
+	TailUUID  string `json:"tail_uuid,omitempty"`
+	TailKind  string `json:"tail_kind,omitempty"`
+}
+
+// linkList implements resourceList interface
+type linkList struct {
+	Items          []link `json:"items"`
+	ItemsAvailable int    `json:"items_available"`
+	Offset         int    `json:"offset"`
+}
+
+func (l linkList) items() []interface{} {
+	var out []interface{}
+	for _, item := range l.Items {
+		out = append(out, item)
+	}
+	return out
+}
+
+func (l linkList) itemsAvailable() int {
+	return l.ItemsAvailable
+}
+
+func (l linkList) offset() int {
+	return l.Offset
+}
+
 func main() {
 	err := doMain()
 	if err != nil {
@@ -32,7 +145,7 @@ func doMain() error {
 	srcPath := flags.String(
 		"path",
 		"",
-		"Local file path containing a CSV-like format.")
+		"Local file path containing a CSV format.")
 
 	userID := flags.String(
 		"user-id",
@@ -56,19 +169,19 @@ func doMain() error {
 
 	// Validations
 	if *retries < 0 {
-		return fmt.Errorf("Retry quantity must be >= 0")
+		return fmt.Errorf("retry quantity must be >= 0")
 	}
 
 	if *srcPath == "" {
-		return fmt.Errorf("Please provide a path to an input file")
+		return fmt.Errorf("please provide a path to an input file")
 	}
-	fileInfo, err := os.Stat(*srcPath)
-	switch {
-	case os.IsNotExist(err):
-		return fmt.Errorf("File not found: %s", *srcPath)
-	case fileInfo.IsDir():
-		return fmt.Errorf("Path provided is not a file: %s", *srcPath)
+
+	// Try opening the input file early, just in case there's problems.
+	f, err := os.Open(*srcPath)
+	if err != nil {
+		return fmt.Errorf("%s", err)
 	}
+	defer f.Close()
 
 	validUserID := false
 	for _, opt := range userIDOpts {
@@ -77,99 +190,101 @@ func doMain() error {
 		}
 	}
 	if !validUserID {
-		return fmt.Errorf("User ID must be one of: %s",
+		return fmt.Errorf("user ID must be one of: %s",
 			strings.Join(userIDOpts, ", "))
 	}
 
 	arv, err := arvadosclient.MakeArvadosClient()
 	if err != nil {
-		return fmt.Errorf("Error setting up arvados client %s", err.Error())
+		return fmt.Errorf("error setting up arvados client %s", err)
 	}
 	arv.Retries = *retries
 
-	log.Printf("Group sync starting. Using '%s' as users id", *userID)
+	log.Printf("Group sync starting. Using %q as users id", *userID)
 
 	// Get the complete user list to minimize API Server requests
-	allUsers := make(map[string]interface{})
+	allUsers := make(map[string]user)
 	userIDToUUID := make(map[string]string) // Index by email or username
-	results := make([]interface{}, 0)
-	err = ListAll(arv, "users", arvadosclient.Dict{}, &results)
+	results, err := ListAll(arv, "users", arvadosclient.Dict{}, &userList{})
 	if err != nil {
-		return fmt.Errorf("Error getting user list from the API Server %s",
-			err.Error())
+		return fmt.Errorf("error getting user list: %s", err)
 	}
 	log.Printf("Found %d users", len(results))
 	for _, item := range results {
-		userMap := item.(map[string]interface{})
-		allUsers[userMap["uuid"].(string)] = userMap
-		userIDToUUID[userMap[*userID].(string)] = userMap["uuid"].(string)
+		u := item.(user)
+		allUsers[u.UUID] = u
+		uID, err := u.GetID(*userID)
+		if err != nil {
+			return err
+		}
+		userIDToUUID[uID] = u.UUID
 		if *verbose {
-			log.Printf("Seen user %s", userMap[*userID].(string))
+			log.Printf("Seen user %q (%s)", u.Username, u.Email)
 		}
 	}
 
 	// Request all UUIDs for groups tagged as remote
-	remoteGroupUUIDs := make(map[string]struct{})
-	results = make([]interface{}, 0)
-	err = ListAll(arv, "links", arvadosclient.Dict{
+	remoteGroupUUIDs := make(map[string]bool)
+	results, err = ListAll(arv, "links", arvadosclient.Dict{
 		"filters": [][]string{
 			{"link_class", "=", "tag"},
 			{"name", "=", groupTag},
 			{"head_kind", "=", "arvados#group"},
 		},
-	}, &results)
+	}, &linkList{})
 	if err != nil {
-		return fmt.Errorf("Error getting remote group UUIDs: %s", err.Error())
+		return fmt.Errorf("error getting remote group UUIDs: %s", err)
 	}
 	for _, item := range results {
-		link := item.(map[string]interface{})
-		remoteGroupUUIDs[link["head_uuid"].(string)] = struct{}{}
+		link := item.(link)
+		remoteGroupUUIDs[link.HeadUUID] = true
 	}
 	// Get remote groups and their members
-	uuidList := make([]string, 0)
+	var uuidList []string
 	for uuid := range remoteGroupUUIDs {
 		uuidList = append(uuidList, uuid)
 	}
-	remoteGroups := make(map[string]arvadosclient.Dict)
+	remoteGroups := make(map[string]*groupInfo)
 	groupNameToUUID := make(map[string]string) // Index by group name
-	results = make([]interface{}, 0)
-	err = ListAll(arv, "groups", arvadosclient.Dict{
+	results, err = ListAll(arv, "groups", arvadosclient.Dict{
 		"filters": [][]interface{}{
 			{"uuid", "in", uuidList},
 		},
-	}, &results)
+	}, &groupList{})
 	if err != nil {
-		return fmt.Errorf("Error getting remote groups by UUID: %s", err.Error())
+		return fmt.Errorf("error getting remote groups by UUID: %s", err)
 	}
 	for _, item := range results {
-		group := item.(map[string]interface{})
-		results := make([]interface{}, 0)
-		err := ListAll(arv, "links", arvadosclient.Dict{
+		group := item.(group)
+		results, err := ListAll(arv, "links", arvadosclient.Dict{
 			"filters": [][]string{
 				{"link_class", "=", "permission"},
 				{"name", "=", "can_read"},
-				{"tail_uuid", "=", group["uuid"].(string)},
+				{"tail_uuid", "=", group.UUID},
 				{"head_kind", "=", "arvados#user"},
 			},
-		}, &results)
+		}, &linkList{})
 		if err != nil {
-			return fmt.Errorf("Error getting member links: %s", err.Error())
+			return fmt.Errorf("error getting member links: %s", err)
 		}
 		// Build a list of user ids (email or username) belonging to this group
-		membersSet := make(map[string]struct{}, 0)
-		for _, linkItem := range results {
-			link := linkItem.(map[string]interface{})
-			memberID := allUsers[link["head_uuid"].(string)].(map[string]interface{})[*userID].(string)
-			membersSet[memberID] = struct{}{}
+		membersSet := make(map[string]bool)
+		for _, item := range results {
+			link := item.(link)
+			memberID, err := allUsers[link.HeadUUID].GetID(*userID)
+			if err != nil {
+				return err
+			}
+			membersSet[memberID] = true
 		}
-		remoteGroups[group["uuid"].(string)] = arvadosclient.Dict{
-			"object":           group,
-			"previous_members": membersSet,
-			"current_members":  make(map[string]struct{}), // Empty set
+		remoteGroups[group.UUID] = &groupInfo{
+			Group:           group,
+			PreviousMembers: membersSet,
+			CurrentMembers:  make(map[string]bool), // Empty set
 		}
 		// FIXME: There's an index (group_name, group.owner_uuid), should we
 		// ask for our own groups tagged as remote? (with own being 'system'?)
-		groupNameToUUID[group["name"].(string)] = group["uuid"].(string)
+		groupNameToUUID[group.Name] = group.UUID
 	}
 	log.Printf("Found %d remote groups", len(remoteGroups))
 
@@ -177,12 +292,6 @@ func doMain() error {
 	membersAdded := 0
 	membersRemoved := 0
 
-	f, err := os.Open(*srcPath)
-	if err != nil {
-		return fmt.Errorf("Error opening file %s: %s", *srcPath, err.Error())
-	}
-	defer f.Close()
-
 	csvReader := csv.NewReader(f)
 	for {
 		record, err := csvReader.Read()
@@ -190,55 +299,55 @@ func doMain() error {
 			break
 		}
 		if err != nil {
-			return fmt.Errorf("Error reading CSV file: %s", err.Error())
+			return fmt.Errorf("error reading %q: %s", *srcPath, err)
 		}
 		groupName := record[0]
 		groupMember := record[1] // User ID (username or email)
 		if _, found := userIDToUUID[groupMember]; !found {
 			// User not present on the system, skip.
-			log.Printf("Warning: there's no user with %s '%s' on the system, skipping.", *userID, groupMember)
+			log.Printf("Warning: there's no user with %s %q on the system, skipping.", *userID, groupMember)
 			continue
 		}
 		if _, found := groupNameToUUID[groupName]; !found {
 			// Group doesn't exist, create and tag it before continuing
-			group := make(map[string]interface{})
+			var group group
 			err := arv.Create("groups", arvadosclient.Dict{
 				"group": arvadosclient.Dict{
 					"name": groupName,
 				},
 			}, &group)
 			if err != nil {
-				return fmt.Errorf("Error creating group named '%s': %s",
-					groupName, err.Error())
+				return fmt.Errorf("error creating group named %q: %s",
+					groupName, err)
 			}
-			groupUUID := group["uuid"].(string)
 			link := make(map[string]interface{})
 			err = arv.Create("links", arvadosclient.Dict{
 				"link": arvadosclient.Dict{
 					"link_class": "tag",
 					"name":       groupTag,
-					"head_uuid":  groupUUID,
+					"head_uuid":  group.UUID,
 				},
 			}, &link)
 			if err != nil {
-				return fmt.Errorf("Error creating tag for group '%s': %s",
-					groupName, err.Error())
+				return fmt.Errorf("error creating tag for group %q: %s",
+					groupName, err)
 			}
 			// Update cached group data
-			groupNameToUUID[groupName] = groupUUID
-			remoteGroups[groupUUID] = arvadosclient.Dict{
-				"object":           group,
-				"previous_members": make(map[string]struct{}), // Empty set
-				"current_members":  make(map[string]struct{}), // Empty set
+			groupNameToUUID[groupName] = group.UUID
+			remoteGroups[group.UUID] = &groupInfo{
+				Group:           group,
+				PreviousMembers: make(map[string]bool), // Empty set
+				CurrentMembers:  make(map[string]bool), // Empty set
 			}
-			groupsCreated = groupsCreated + 1
+			groupsCreated++
 		}
 		// Both group & user exist, check if user is a member
 		groupUUID := groupNameToUUID[groupName]
-		previousMembersSet := remoteGroups[groupUUID]["previous_members"].(map[string]struct{})
-		currentMembersSet := remoteGroups[groupUUID]["current_members"].(map[string]struct{})
-		if !(contains(previousMembersSet, groupMember) ||
-			contains(currentMembersSet, groupMember)) {
+		gi := remoteGroups[groupUUID]
+		if !gi.PreviousMembers[groupMember] && !gi.CurrentMembers[groupMember] {
+			if *verbose {
+				log.Printf("Adding %q to group %q", groupMember, groupName)
+			}
 			// User wasn't a member, but should.
 			link := make(map[string]interface{})
 			err := arv.Create("links", arvadosclient.Dict{
@@ -250,45 +359,46 @@ func doMain() error {
 				},
 			}, &link)
 			if err != nil {
-				return fmt.Errorf("Error adding user '%s' to group '%s': %s",
-					groupMember, groupName, err.Error())
+				return fmt.Errorf("error adding user %q to group %q: %s",
+					groupMember, groupName, err)
 			}
-			membersAdded = membersAdded + 1
+			membersAdded++
 		}
-		currentMembersSet[groupMember] = struct{}{}
+		gi.CurrentMembers[groupMember] = true
 	}
 
 	// Remove previous members not listed on this run
 	for groupUUID := range remoteGroups {
-		previousMembersSet := remoteGroups[groupUUID]["previous_members"].(map[string]struct{})
-		currentMembersSet := remoteGroups[groupUUID]["current_members"].(map[string]struct{})
-		evictedMembersSet := subtract(previousMembersSet, currentMembersSet)
-		groupName := remoteGroups[groupUUID]["object"].(map[string]interface{})["name"]
-		if len(evictedMembersSet) > 0 {
-			log.Printf("Removing %d users from group '%s'", len(evictedMembersSet), groupName)
+		gi := remoteGroups[groupUUID]
+		evictedMembers := subtract(gi.PreviousMembers, gi.CurrentMembers)
+		groupName := gi.Group.Name
+		if len(evictedMembers) > 0 {
+			log.Printf("Removing %d users from group %q", len(evictedMembers), groupName)
 		}
-		for evictedUser := range evictedMembersSet {
-			links := make([]interface{}, 0)
-			err := ListAll(arv, "links", arvadosclient.Dict{
+		for evictedUser := range evictedMembers {
+			links, err := ListAll(arv, "links", arvadosclient.Dict{
 				"filters": [][]string{
 					{"link_class", "=", "permission"},
 					{"name", "=", "can_read"},
 					{"tail_uuid", "=", groupUUID},
 					{"head_uuid", "=", userIDToUUID[evictedUser]},
 				},
-			}, &links)
+			}, &linkList{})
 			if err != nil {
-				return fmt.Errorf("Error getting links needed to remove user '%s' from group '%s': %s", evictedUser, groupName, err.Error())
+				return fmt.Errorf("error getting links needed to remove user %q from group %q: %s", evictedUser, groupName, err)
 			}
-			for _, link := range links {
-				linkUUID := link.(map[string]interface{})["uuid"].(string)
-				l := make(map[string]interface{})
-				err := arv.Delete("links", linkUUID, arvadosclient.Dict{}, &l)
+			for _, item := range links {
+				link := item.(link)
+				var l map[string]interface{}
+				if *verbose {
+					log.Printf("Removing %q from group %q", evictedUser, gi.Group.Name)
+				}
+				err := arv.Delete("links", link.UUID, arvadosclient.Dict{}, &l)
 				if err != nil {
-					return fmt.Errorf("Error removing user '%s' from group '%s': %s", evictedUser, groupName, err.Error())
+					return fmt.Errorf("error removing user %q from group %q: %s", evictedUser, groupName, err)
 				}
 			}
-			membersRemoved = membersRemoved + 1
+			membersRemoved++
 		}
 	}
 	log.Printf("Groups created: %d, members added: %d, members removed: %d", groupsCreated, membersAdded, membersRemoved)
@@ -297,42 +407,33 @@ func doMain() error {
 }
 
 // ListAll : Adds all objects of type 'resource' to the 'output' list
-func ListAll(arv *arvadosclient.ArvadosClient, resource string, parameters arvadosclient.Dict, output *[]interface{}) (err error) {
-	// Default limit value
+func ListAll(arv *arvadosclient.ArvadosClient, resource string, parameters arvadosclient.Dict, rl resourceList) (allItems []interface{}, err error) {
 	if _, ok := parameters["limit"]; !ok {
-		parameters["limit"] = 1000
+		// Default limit value: use the maximum page size the server allows
+		parameters["limit"] = 1<<31 - 1
 	}
 	offset := 0
 	itemsAvailable := parameters["limit"].(int)
-	for len(*output) < itemsAvailable {
-		results := make(arvadosclient.Dict)
+	for len(allItems) < itemsAvailable {
 		parameters["offset"] = offset
-		err = arv.List(resource, parameters, &results)
+		err = arv.List(resource, parameters, &rl)
 		if err != nil {
-			return err
+			return allItems, err
 		}
-		if value, ok := results["items"]; ok {
-			items := value.([]interface{})
-			for _, item := range items {
-				*output = append(*output, item)
-			}
-			offset = int(results["offset"].(float64)) + len(items)
+		for _, i := range rl.items() {
+			allItems = append(allItems, i)
 		}
-		itemsAvailable = int(results["items_available"].(float64))
+		offset = rl.offset() + len(rl.items())
+		itemsAvailable = rl.itemsAvailable()
 	}
-	return nil
-}
-
-func contains(set map[string]struct{}, element string) bool {
-	_, found := set[element]
-	return found
+	return allItems, nil
 }
 
-func subtract(setA map[string]struct{}, setB map[string]struct{}) map[string]struct{} {
-	result := make(map[string]struct{})
+func subtract(setA map[string]bool, setB map[string]bool) map[string]bool {
+	result := make(map[string]bool)
 	for element := range setA {
-		if !contains(setB, element) {
-			result[element] = struct{}{}
+		if !setB[element] {
+			result[element] = true
 		}
 	}
 	return result

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list