Is2911/fix delete project (#2967)

Makefile

@@ -231,20 +231,24 @@ rows="%-24s | %90s | %12s | %12s\n";\
 TableWidth=140;\
 printf "%24s | %90s | %12s | %12s\n" Name Endpoint User Password;\
 printf "%.$${TableWidth}s\n" "$$separator";\
-printf "$$rows" 'oSparc platform' 'http://$(get_my_ip).nip.io:9081';\
-printf "$$rows" 'oSparc web API doc' 'http://$(get_my_ip).nip.io:9081/dev/doc';\
-printf "$$rows" 'oSparc public API doc' 'http://$(get_my_ip).nip.io:8006/dev/doc';\
-printf "$$rows" 'Postgres DB' 'http://$(get_my_ip).nip.io:18080/?pgsql=postgres&username='$${POSTGRES_USER}'&db='$${POSTGRES_DB}'&ns=public' $${POSTGRES_USER} $${POSTGRES_PASSWORD};\
-printf "$$rows" Portainer 'http://$(get_my_ip).nip.io:9000' admin adminadmin;\
-printf "$$rows" Redis 'http://$(get_my_ip).nip.io:18081';\
+printf "$$rows" "oSparc platform" "http://$(get_my_ip).nip.io:9081";\
+printf "$$rows" "oSparc web API doc" "http://$(get_my_ip).nip.io:9081/dev/doc";\
+printf "$$rows" "oSparc public API doc" "http://$(get_my_ip).nip.io:8006/dev/doc";\
+printf "$$rows" "Postgres DB" "http://$(get_my_ip).nip.io:18080/?pgsql=postgres&username="$${POSTGRES_USER}"&db="$${POSTGRES_DB}"&ns=public" $${POSTGRES_USER} $${POSTGRES_PASSWORD};\
+printf "$$rows" "Portainer" "http://$(get_my_ip).nip.io:9000" admin adminadmin;\
+printf "$$rows" "Redis" "http://$(get_my_ip).nip.io:18081";\
 printf "$$rows" "Dask Dashboard" "http://$(get_my_ip).nip.io:8787";\
 printf "$$rows" "Docker Registry" "$${REGISTRY_URL}" $${REGISTRY_USER} $${REGISTRY_PW};\
 printf "$$rows" "Rabbit Dashboard" "http://$(get_my_ip).nip.io:15762" admin adminadmin;\
 printf "$$rows" "Traefik Dashboard" "http://$(get_my_ip).nip.io:8080/dashboard/";\
+printf "$$rows" "Storage S3 Filestash" "http://$(get_my_ip).nip.io:9002" 12345678 12345678;\
+printf "$$rows" "Storage S3 Minio" "http://$(get_my_ip).nip.io:9001" 12345678 12345678;\
+
 printf "\n%s\n" "⚠️ if a DNS is not used (as displayed above), the interactive services started via dynamic-sidecar";\
 echo "⚠️ will not be shown. The frontend accesses them via the uuid.services.YOUR_IP.nip.io:9081";
 endef
 
+
 show-endpoints:
 	@$(_show_endpoints)
 

services/storage/src/simcore_service_storage/handlers.py

@@ -10,7 +10,7 @@
 from servicelib.aiohttp.application_keys import APP_CONFIG_KEY
 from servicelib.aiohttp.rest_utils import extract_and_validate
 
-from ._meta import __version__, api_vtag
+from ._meta import api_vtag
 from .access_layer import InvalidFileIdentifier
 from .constants import APP_DSM_KEY, DATCORE_STR, SIMCORE_S3_ID, SIMCORE_S3_STR
 from .db_tokens import get_api_token_and_secret
@@ -257,7 +257,7 @@ async def _go():
                 except asyncio.TimeoutError:
                     log.error("Sync metadata table timed out (%s seconds)", timeout)
 
-            asyncio.create_task(_go(), name="f&f sync_task")
+            asyncio.create_task(_go(), name="fire&forget sync_task")
         else:
             sync_results = await sync_coro
 

services/web/server/requirements/_test.in

@@ -7,13 +7,21 @@
 #
 --constraint _base.txt
 
-# testing
 
-# fixtures
-hypothesis
+aioresponses
+alembic
+click
+codecov
 coverage
+coveralls
+docker
 Faker
 flaky
+hypothesis
+jsonschema
+openapi-spec-validator
+ptvsd
+pylint
 pytest
 pytest-aiohttp  # incompatible with pytest-asyncio. See https://github.com/pytest-dev/pytest-asyncio/issues/76
 pytest-cov
@@ -23,21 +31,7 @@ pytest-instafail
 pytest-mock
 pytest-runner
 pytest-sugar
-
-# helpers
-aioresponses
-alembic
-click
-docker
-jsonschema
-openapi-spec-validator
 python-dotenv
 redis
 tenacity
 websockets
-
-# tools
-codecov
-coveralls
-ptvsd
-pylint

services/web/server/setup.cfg

@@ -12,6 +12,8 @@ tag = False
 
 [tool:pytest]
 addopts = --strict-markers
+# switching to 'auto' for the sake of pytest-aiohttp backward compatibility
+asyncio_mode=auto
 markers =
 	slow: marks tests as slow (deselect with '-m "not slow"')
-	acceptance_test: marks tests as 'acceptance tests' i.e. does the system do what the user expects? Typically those are workflows.
+	acceptance_test: "marks tests as 'acceptance tests' i.e. does the system do what the user expects? Typically those are workflows."

services/web/server/src/simcore_service_webserver/exporter/formatters/formatter_v1.py

@@ -8,6 +8,7 @@
 from pathlib import Path
 from pprint import pformat
 from typing import Any, Deque, Dict, List, Optional, Tuple
+from uuid import UUID
 
 import aiofiles
 from aiohttp import ClientSession, ClientTimeout, web
@@ -16,7 +17,7 @@
 from models_library.utils.nodes import compute_node_hash, project_node_io_payload_cb
 
 from ...director_v2_api import create_or_update_pipeline
-from ...projects.projects_api import delete_project, get_project_for_user
+from ...projects.projects_api import get_project_for_user, submit_delete_project_task
 from ...projects.projects_db import APP_PROJECT_DBAPI
 from ...projects.projects_exceptions import ProjectsException
 from ...storage_handlers import (
@@ -419,7 +420,9 @@ async def import_files_and_validate_project(
             "Removing project %s, because there was an error while importing it."
         )
         try:
-            await delete_project(app=app, project_uuid=project_uuid, user_id=user_id)
+            await submit_delete_project_task(
+                app=app, project_uuid=UUID(project_uuid), user_id=user_id
+            )
         except ProjectsException:
             # no need to raise an error here
             log.exception(

services/web/server/src/simcore_service_webserver/garbage_collector_core.py

@@ -2,6 +2,7 @@
 
 """
 
+import asyncio
 import logging
 from itertools import chain
 from typing import Any, Dict, List, Optional, Set, Tuple
@@ -16,23 +17,24 @@
 from . import director_v2_api, users_exceptions
 from .director.director_exceptions import DirectorException, ServiceNotFoundError
 from .garbage_collector_settings import GUEST_USER_RC_LOCK_FORMAT
-from .garbage_collector_utils import (
-    get_new_project_owner_gid,
-    remove_user,
-    replace_current_owner,
-)
+from .garbage_collector_utils import get_new_project_owner_gid, replace_current_owner
 from .projects.projects_api import (
-    delete_project,
     get_project_for_user,
     get_workbench_node_ids_from_project_uuid,
     is_node_id_present_in_any_project_workbench,
-    remove_project_interactive_services,
+    remove_project_dynamic_services,
+    submit_delete_project_task,
 )
 from .projects.projects_db import APP_PROJECT_DBAPI
-from .projects.projects_exceptions import ProjectNotFoundError
+from .projects.projects_exceptions import ProjectDeleteError, ProjectNotFoundError
 from .redis import get_redis_lock_manager_client
 from .resource_manager.registry import RedisResourceRegistry, get_registry
-from .users_api import get_guest_user_ids_and_names, get_user, get_user_role
+from .users_api import (
+    delete_user,
+    get_guest_user_ids_and_names,
+    get_user,
+    get_user_role,
+)
 from .users_exceptions import UserNotFoundError
 
 logger = logging.getLogger(__name__)
@@ -183,7 +185,7 @@ async def remove_disconnected_user_resources(
                     # inform that the project can be closed on the backend side
                     #
                     try:
-                        await remove_project_interactive_services(
+                        await remove_project_dynamic_services(
                             user_id=int(dead_key["user_id"]),
                             project_uuid=resource_value,
                             app=app,
@@ -417,44 +419,7 @@ async def remove_orphaned_services(
     logger.debug("Finished orphaned services removal")
 
 
-async def remove_guest_user_with_all_its_resources(
-    app: web.Application, user_id: int
-) -> None:
-    """Removes a GUEST user with all its associated projects and S3/MinIO files"""
-
-    try:
-        user_role: UserRole = await get_user_role(app, user_id)
-        if user_role > UserRole.GUEST:
-            # NOTE: This acts as a protection barrier to avoid removing resources to more
-            # priviledge users
-            return
-
-        logger.debug(
-            "Deleting all projects of user with %s because it is a GUEST",
-            f"{user_id=}",
-        )
-        await remove_all_projects_for_user(app=app, user_id=user_id)
-
-        logger.debug(
-            "Deleting user %s because it is a GUEST",
-            f"{user_id=}",
-        )
-        await remove_user(app=app, user_id=user_id)
-
-    except (
-        DatabaseError,
-        asyncpg.exceptions.PostgresError,
-        ProjectNotFoundError,
-        UserNotFoundError,
-    ) as error:
-        logger.warning(
-            "Failure in database while removing user (%s) and its resources with %s",
-            f"{user_id=}",
-            f"{error}",
-        )
-
-
-async def remove_all_projects_for_user(app: web.Application, user_id: int) -> None:
+async def _delete_all_projects_for_user(app: web.Application, user_id: int) -> None:
     """
     Goes through all the projects and will try to remove them but first it will check if
     the project is shared with others.
@@ -491,6 +456,8 @@ async def remove_all_projects_for_user(app: web.Application, user_id: int) -> No
         f"{user_project_uuids=}",
     )
 
+    delete_tasks: List[asyncio.Task] = []
+
     for project_uuid in user_project_uuids:
         try:
             project: Dict = await get_project_for_user(
@@ -499,11 +466,12 @@ async def remove_all_projects_for_user(app: web.Application, user_id: int) -> No
                 user_id=user_id,
                 include_templates=True,
             )
-        except web.HTTPNotFound:
+        except (web.HTTPNotFound, ProjectNotFoundError) as err:
             logger.warning(
-                "Could not find project %s for user with %s to be removed. Skipping.",
+                "Could not find project %s for user with %s to be removed: %s. Skipping.",
                 f"{project_uuid=}",
                 f"{user_id=}",
+                f"{err}",
             )
             continue
 
@@ -525,8 +493,9 @@ async def remove_all_projects_for_user(app: web.Application, user_id: int) -> No
                     f"{project_uuid=}",
                     f"{user_id=}",
                 )
-
-                await delete_project(app, project_uuid, user_id)
+                task = await submit_delete_project_task(app, project_uuid, user_id)
+                assert task  # nosec
+                delete_tasks.append(task)
 
             except ProjectNotFoundError:
                 logging.warning(
@@ -551,3 +520,45 @@ async def remove_all_projects_for_user(app: web.Application, user_id: int) -> No
                 new_project_owner_gid=new_project_owner_gid,
                 project=project,
             )
+
+    # NOTE: ensures all delete_task tasks complete or fails fast
+    # can raise ProjectDeleteError, CancellationError
+    await asyncio.gather(*delete_tasks)
+
+
+async def remove_guest_user_with_all_its_resources(
+    app: web.Application, user_id: int
+) -> None:
+    """Removes a GUEST user with all its associated projects and S3/MinIO files"""
+
+    try:
+        user_role: UserRole = await get_user_role(app, user_id)
+        if user_role > UserRole.GUEST:
+            # NOTE: This acts as a protection barrier to avoid removing resources to more
+            # priviledge users
+            return
+
+        logger.debug(
+            "Deleting all projects of user with %s because it is a GUEST",
+            f"{user_id=}",
+        )
+        await _delete_all_projects_for_user(app=app, user_id=user_id)
+
+        logger.debug(
+            "Deleting user %s because it is a GUEST",
+            f"{user_id=}",
+        )
+        await delete_user(app, user_id)
+
+    except (
+        DatabaseError,
+        asyncpg.exceptions.PostgresError,
+        ProjectNotFoundError,
+        UserNotFoundError,
+        ProjectDeleteError,
+    ) as error:
+        logger.warning(
+            "Failed to delete user %s and its resources: %s",
+            f"{user_id=}",
+            f"{error}",
+        )

services/web/server/src/simcore_service_webserver/garbage_collector_task.py

@@ -15,7 +15,7 @@
 logger = logging.getLogger(__name__)
 
 
-GC_TASK_NAME = f"{__name__}.collect_garbage_periodically"
+GC_TASK_NAME = f"background-task.{__name__}.collect_garbage_periodically"
 GC_TASK_CONFIG = f"{GC_TASK_NAME}.config"
 
 

services/web/server/src/simcore_service_webserver/garbage_collector_utils.py

@@ -12,7 +12,7 @@
 from .groups_api import get_group_from_gid
 from .projects.projects_db import APP_PROJECT_DBAPI, ProjectAccessRights
 from .projects.projects_exceptions import ProjectNotFoundError
-from .users_api import delete_user, get_user, get_user_id_from_gid
+from .users_api import get_user, get_user_id_from_gid
 from .users_exceptions import UserNotFoundError
 from .users_to_groups_api import get_users_for_gid
 
@@ -31,7 +31,7 @@ async def _fetch_new_project_owner_from_groups(
     for group_gid in standard_groups.keys():
         # remove the current owner from the bunch
         target_group_users = await get_users_for_gid(app=app, gid=group_gid) - {user_id}
-        logger.error("Found group users '%s'", target_group_users)
+        logger.info("Found group users '%s'", target_group_users)
 
         for possible_user_id in target_group_users:
             # check if the possible_user is still present in the db
@@ -63,7 +63,7 @@ async def get_new_project_owner_gid(
     # A Set[str] is prefered over Set[int] because access_writes
     # is a Dict with only key,valus in {str, None}
     other_users_access_rights: Set[str] = set(access_rights.keys()) - {
-        str(user_primary_gid)
+        f"{user_primary_gid}"
     }
     logger.debug(
         "Processing other user and groups access rights '%s'",
@@ -151,11 +151,11 @@ async def replace_current_owner(
     # unseting the project owner and saving the project back
     project["prj_owner"] = int(new_project_owner_id)
     # removing access rights entry
-    del project["accessRights"][str(user_primary_gid)]
+    del project["accessRights"][f"{user_primary_gid}"]
     project["accessRights"][
-        str(new_project_owner_gid)
+        f"{new_project_owner_gid}"
     ] = ProjectAccessRights.OWNER.value
-    logger.error("Syncing back project %s", project)
+    logger.info("Syncing back project %s", project)
 
     # syncing back project data
     try:
@@ -173,18 +173,3 @@ async def replace_current_owner(
             "Could not remove old owner and replaced it with user %s",
             new_project_owner_id,
         )
-
-
-async def remove_user(app: web.Application, user_id: UserID) -> None:
-    """Tries to remove a user, if the users still exists a warning message will be displayed"""
-    try:
-        await delete_user(app, user_id)
-    except (
-        DatabaseError,
-        asyncpg.exceptions.PostgresError,
-        ProjectNotFoundError,
-        UserNotFoundError,
-    ) as err:
-        logger.warning(
-            "User '%s' still has some projects, could not be deleted [%s]", user_id, err
-        )