From 8e8b9166245393ede5cdfd6061b06475c7073661 Mon Sep 17 00:00:00 2001
From: Jim Laney <jlaney@uw.edu>
Date: Mon, 9 Dec 2024 07:27:39 -0800
Subject: [PATCH] Better exception handling for these management commands

---
 Dockerfile                                    |  2 +-
 .../commands/check_role_permissions.py        | 12 +++++++-----
 .../commands/remove_unauthorized_admins.py    | 19 +++++++++++++------
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/Dockerfile b/Dockerfile
index b7f4ed5b..926d5abd 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,4 +1,4 @@
-ARG DJANGO_CONTAINER_VERSION=2.0.5
+ARG DJANGO_CONTAINER_VERSION=2.0.6
 
 FROM us-docker.pkg.dev/uwit-mci-axdd/containers/django-container:${DJANGO_CONTAINER_VERSION} AS app-container
 
diff --git a/sis_provisioner/management/commands/check_role_permissions.py b/sis_provisioner/management/commands/check_role_permissions.py
index be844674..90efad77 100644
--- a/sis_provisioner/management/commands/check_role_permissions.py
+++ b/sis_provisioner/management/commands/check_role_permissions.py
@@ -6,6 +6,9 @@
 from django.core.management.base import CommandError
 from sis_provisioner.management.commands import SISProvisionerCommand
 from sis_provisioner.models.admin import RoleCache
+from logging import getLogger
+
+logger = getLogger(__name__)
 
 
 class Command(SISProvisionerCommand):
@@ -20,9 +23,8 @@ def handle(self, *args, **options):
             except Exception as ex:
                 raise CommandError(ex)
 
-        self.update_job()
-
         if len(notify_accounts):
-            raise CommandError(
-                'Permissions changed for accounts: {}'.format(
-                    ', '.join(notify_accounts)))
+            log_accounts = ', '.join(notify_accounts)
+            logger.info(f'Permissions changed for accounts: {log_accounts}')
+
+        self.update_job()
diff --git a/sis_provisioner/management/commands/remove_unauthorized_admins.py b/sis_provisioner/management/commands/remove_unauthorized_admins.py
index 0d00d681..e1ff4819 100644
--- a/sis_provisioner/management/commands/remove_unauthorized_admins.py
+++ b/sis_provisioner/management/commands/remove_unauthorized_admins.py
@@ -3,13 +3,14 @@
 
 
 from django.conf import settings
+from restclients_core.exceptions import DataFailureException
 from sis_provisioner.models.admin import Admin
 from sis_provisioner.dao.canvas import (
     get_account_by_id, get_all_sub_accounts, get_admins, delete_admin)
 from sis_provisioner.management.commands import SISProvisionerCommand
 from logging import getLogger
 
-logger = getLogger('sis_provisioner.dao.astra')
+logger = getLogger(__name__)
 
 
 class Command(SISProvisionerCommand):
@@ -34,15 +35,21 @@ def handle(self, *args, **options):
         for account in accounts:
             account_id = account.account_id
 
-            for admin in get_admins(account_id):
+            try:
+                admins = get_admins(account_id)
+            except DataFailureException as ex:
+                logger.error(
+                    f'get_admins failed for account "{account_id}": {ex}')
+                continue
+
+            for admin in admins:
                 if not Admin.objects.verify_canvas_admin(admin, account_id):
                     if options.get('commit'):
                         delete_admin(
                             account_id, admin.user.user_id, admin.role)
 
-                    logger.info((
-                        'REMOVE UNAUTHORIZED ADMIN "{}", account: "{}", '
-                        'role: "{}"').format(
-                            admin.user.login_id, account_id, admin.role))
+                    logger.info(
+                        f'REMOVE UNAUTHORIZED ADMIN "{admin.user.login_id}", '
+                        f'account: "{account_id}", role: "{admin.role}"')
 
         self.update_job()