From 285750d0627e3539ddaf5169fa584e669eabf6df Mon Sep 17 00:00:00 2001
From: Nathan Franklin <nathan.franklin@gmail.com>
Date: Thu, 29 Sep 2022 12:56:44 -0500
Subject: [PATCH] Task/DES-2231 handle potential listing errors (#105)

* User logger from geoapi.log

* Add missing db_session.commit after project deletion

* Add another delete test

Note that this test doesn't catch the regression.  Not sure why the
db_session.commit isn't needed in the unit test. I don't see it but its
like the db_session is configured for auto-commit

* Handle when directory listing fails

* Refactor refresh to catch error when importing project but then continue with rest of projects

Before, we just stopped instead of working on the rest of the projects

* Update geoapi/tests/api_tests/test_projects_service.py

Co-authored-by: Ian Park <contact@duckonomy.com>

Co-authored-by: Ian Park <contact@duckonomy.com>
---
 geoapi/tasks/external_data.py | 31 +++++++++++++++++--------------
 geoapi/utils/agave.py         |  9 +++++++++
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/geoapi/tasks/external_data.py b/geoapi/tasks/external_data.py
index 0af8fdef..6285e47f 100644
--- a/geoapi/tasks/external_data.py
+++ b/geoapi/tasks/external_data.py
@@ -11,7 +11,8 @@
 from geoapi.celery_app import app
 from geoapi.exceptions import InvalidCoordinateReferenceSystem, MissingServiceAccount
 from geoapi.models import User, Project, ProjectUser, ObservableDataProject, Task
-from geoapi.utils.agave import AgaveUtils, SystemUser, get_system_users, get_metadata_using_service_account, AgaveFileGetError
+from geoapi.utils.agave import (AgaveUtils, SystemUser, get_system_users, get_metadata_using_service_account,
+                                AgaveFileGetError, AgaveListingError)
 from geoapi.log import logger
 from geoapi.services.features import FeaturesService
 from geoapi.services.imports import ImportsService
@@ -224,7 +225,13 @@ def import_from_agave(tenant_id: str, userId: int, systemId: str, path: str, pro
                                                                               systemId,
                                                                               path,
                                                                               user.username))
-    listing = client.listing(systemId, path)
+    try:
+        listing = client.listing(systemId, path)
+    except AgaveListingError:
+        logger.exception(f"Unable to perform file listing on {systemId}/{path} when importing for project:{projectId}")
+        NotificationsService.create(user, "error", f"Error importing as unable to access {systemId}/{path}")
+        return
+
     # First item is always a reference to self
     files_in_directory = listing[1:]
     filenames_in_directory = [str(f.path) for f in files_in_directory]
@@ -324,16 +331,13 @@ def import_from_agave(tenant_id: str, userId: int, systemId: str, path: str, pro
 @app.task()
 def refresh_observable_projects():
     start_time = time.time()
-    try:
-        obs = db_session.query(ObservableDataProject).all()
-        for i, o in enumerate(obs):
+    obs = db_session.query(ObservableDataProject).all()
+    for i, o in enumerate(obs):
+        try:
             # we need a user with a jwt for importing
             importing_user = next((u for u in o.project.users if u.jwt))
-            logger.info("Refreshing observable project ({}/{}): observer:{} system:{} path:{}".format(i,
-                                                                                                      len(obs),
-                                                                                                      importing_user,
-                                                                                                      o.system_id,
-                                                                                                      o.path))
+            logger.info(f"Refreshing observable project ({i}/{len(obs)}): observer:{importing_user} "
+                        f"system:{o.system_id} path:{o.path} project:{o.project.id}")
 
             # we need to add any users who have been added to the project/system or update if their admin-status
             # has changed
@@ -371,12 +375,11 @@ def refresh_observable_projects():
             # perform the importing
             if o.watch_content:
                 import_from_agave(o.project.tenant_id, importing_user.id, o.system_id, o.path, o.project.id)
-    except Exception:  # noqa: E722
-        logger.exception("Unhandled exception when importing observable project")
-        db_session.rollback()
+        except Exception:  # noqa: E722
+            logger.exception(f"Unhandled exception when importing observable project:{o.project.id}")
+            db_session.rollback()
 
     total_time = time.time() - start_time
-    logger.info(f"{total_time}")
     logger.info("refresh_observable_projects completed. "
                 "Elapsed time {}".format(datetime.timedelta(seconds=total_time)))
 
diff --git a/geoapi/utils/agave.py b/geoapi/utils/agave.py
index 8098279e..a1a604c8 100644
--- a/geoapi/utils/agave.py
+++ b/geoapi/utils/agave.py
@@ -22,6 +22,12 @@
 SLEEP_SECONDS_BETWEEN_RETRY = 2
 
 
+class AgaveListingError(Exception):
+    '''' Unable to list directory from agave
+    '''
+    pass
+
+
 class AgaveFileGetError(Exception):
     '''' Unable to fetch file from agave
     '''
@@ -113,6 +119,9 @@ def systemsRolesGet(self, systemId: str) -> Dict:
     def listing(self, systemId: str, path: str) -> List[AgaveFileListing]:
         url = quote('/files/listings/system/{}/{}?limit=10000'.format(systemId, path))
         resp = self.get(url)
+        if resp.status_code != 200:
+            raise AgaveListingError(f"Unable to perform files listing of {systemId}/{path}. "
+                                    f"Status code: {resp.status_code}")
         listing = resp.json()
         out = [AgaveFileListing(d) for d in listing["result"]]
         return out