Code cleanup and style improvements in team sync

data/migrations/versions/be8d1c402ce0_add_teamsync_table.py

@@ -1,14 +1,14 @@
 """Add TeamSync table
 
 Revision ID: be8d1c402ce0
-Revises: e2894a3a3c19
+Revises: a6c463dfb9fe
 Create Date: 2017-02-23 13:34:52.356812
 
 """
 
 # revision identifiers, used by Alembic.
 revision = 'be8d1c402ce0'
-down_revision = 'e2894a3a3c19'
+down_revision = 'a6c463dfb9fe'
 
 from alembic import op
 import sqlalchemy as sa

data/users/externalldap.py

@@ -274,8 +274,7 @@ class LDAPUsers(FederatedUsers):
     if err is not None:
       return (False, err)
 
-    results = list(it)
+    if not list(it):
-    if not results:
       return (False, 'Group does not exist or is empty')
 
     return (True, None)
@@ -340,6 +339,6 @@ class LDAPUsers(FederatedUsers):
               # No additional results.
               break
           else:
-            # Pagintation is not supported.
+            # Pagination is not supported.
             logger.debug('Pagination is not supported for this LDAP server')
             break

data/users/teamsync.py

@@ -6,6 +6,9 @@ from data import model
 logger = logging.getLogger(__name__)
 
 
+MAX_TEAMS_PER_ITERATION = 500
+
+
 def sync_teams_to_groups(authentication, stale_cutoff):
   """ Performs team syncing by looking up any stale team(s) found, and performing the sync
       operation on them.
@@ -13,7 +16,7 @@ def sync_teams_to_groups(authentication, stale_cutoff):
   logger.debug('Looking up teams to sync to groups')
 
   sync_team_tried = set()
-  while True:
+  while len(sync_team_tried) < MAX_TEAMS_PER_ITERATION:
     # Find a stale team.
     stale_team_sync = model.team.get_stale_team(stale_cutoff)
     if not stale_team_sync:
@@ -38,7 +41,8 @@ def sync_team(authentication, stale_team_sync):
   """
   sync_config = json.loads(stale_team_sync.config)
   logger.info('Syncing team `%s` under organization %s via %s (#%s)', stale_team_sync.team.name,
-              stale_team_sync.team.organization.username, sync_config, stale_team_sync.team_id)
+              stale_team_sync.team.organization.username, sync_config, stale_team_sync.team_id,
+              extra={'team': stale_team_sync.team_id, 'sync_config': sync_config})
 
   # Load all the existing members of the team in Quay that are bound to the auth service.
   existing_users = model.team.list_federated_team_members(stale_team_sync.team,
@@ -46,7 +50,9 @@ def sync_team(authentication, stale_team_sync):
 
   logger.debug('Existing membership of %s for team `%s` under organization %s via %s (#%s)',
                len(existing_users), stale_team_sync.team.name,
-               stale_team_sync.team.organization.username, sync_config, stale_team_sync.team_id)
+               stale_team_sync.team.organization.username, sync_config, stale_team_sync.team_id,
+               extra={'team': stale_team_sync.team_id, 'sync_config': sync_config,
+                      'existing_member_count': len(existing_users)})
 
   # Load all the members of the team from the authenication system.
   (member_iterator, err) = authentication.iterate_group_members(sync_config)
@@ -68,7 +74,9 @@ def sync_team(authentication, stale_team_sync):
       logger.debug('Member %s already in team `%s` under organization %s via %s (#%s)',
                    member_info.username, stale_team_sync.team.name,
                    stale_team_sync.team.organization.username, sync_config,
-                   stale_team_sync.team_id)
+                   stale_team_sync.team_id,
+                   extra={'team': stale_team_sync.team_id, 'sync_config': sync_config,
+                          'member': member_info.username})
 
       group_membership.add(existing_users[member_info.username])
       continue
@@ -77,7 +85,9 @@ def sync_team(authentication, stale_team_sync):
     (quay_user, err) = authentication.get_federated_user(member_info)
     if err is not None:
       logger.error('Could not link external user %s to an internal user: %s',
-                   member_info.username, err)
+                   member_info.username, err,
+                   extra={'team': stale_team_sync.team_id, 'sync_config': sync_config,
+                          'member': member_info.username, 'error': err})
       continue
 
     # Add the user to the membership set.
@@ -88,7 +98,9 @@ def sync_team(authentication, stale_team_sync):
       logger.info('Adding member %s to team `%s` under organization %s via %s (#%s)',
                   quay_user.username, stale_team_sync.team.name,
                   stale_team_sync.team.organization.username, sync_config,
-                  stale_team_sync.team_id)
+                  stale_team_sync.team_id,
+                  extra={'team': stale_team_sync.team_id, 'sync_config': sync_config,
+                         'member': quay_user.username})
 
       model.team.add_user_to_team(quay_user, stale_team_sync.team)
     except model.UserAlreadyInTeam:
@@ -103,18 +115,21 @@ def sync_team(authentication, stale_team_sync):
     logger.debug('Another worker synced team `%s` under organization %s via %s (#%s)',
                  stale_team_sync.team.name,
                  stale_team_sync.team.organization.username, sync_config,
-                 stale_team_sync.team_id)
+                 stale_team_sync.team_id,
+                 extra={'team': stale_team_sync.team_id, 'sync_config': sync_config})
     return True
 
   # Delete any team members not found in the backing auth system.
   logger.debug('Deleting stale members for team `%s` under organization %s via %s (#%s)',
                stale_team_sync.team.name, stale_team_sync.team.organization.username,
-               sync_config, stale_team_sync.team_id)
+               sync_config, stale_team_sync.team_id,
+               extra={'team': stale_team_sync.team_id, 'sync_config': sync_config})
 
   deleted = model.team.delete_members_not_present(stale_team_sync.team, group_membership)
 
   # Done!
   logger.info('Finishing sync for team `%s` under organization %s via %s (#%s): %s deleted',
               stale_team_sync.team.name, stale_team_sync.team.organization.username,
-              sync_config, stale_team_sync.team_id, deleted)
+              sync_config, stale_team_sync.team_id, deleted,
+              extra={'team': stale_team_sync.team_id, 'sync_config': sync_config})
   return True