address comments

Author: Panaetius

components/renku_data_services/project/blueprints.py

@@ -6,6 +6,7 @@
 from sanic import HTTPResponse, Request, json
 from sanic.response import JSONResponse
 from sanic_ext import validate
+from ulid import ULID
 
 import renku_data_services.base_models as base_models
 from renku_data_services.authz.models import Member, Role, Visibility
@@ -110,7 +111,7 @@ def get_one(self) -> BlueprintFactoryResponse:
         @authenticate(self.authenticator)
         @validate_path_project_id
         async def _get_one(request: Request, user: base_models.APIUser, project_id: str) -> JSONResponse | HTTPResponse:
-            project = await self.project_repo.get_project(user=user, project_id=project_id)
+            project = await self.project_repo.get_project(user=user, project_id=ULID.from_str(project_id))
 
             etag = request.headers.get("If-None-Match")
             if project.etag is not None and project.etag == etag:
@@ -176,7 +177,7 @@ def delete(self) -> BlueprintFactoryResponse:
         @only_authenticated
         @validate_path_project_id
         async def _delete(_: Request, user: base_models.APIUser, project_id: str) -> HTTPResponse:
-            await self.project_repo.delete_project(user=user, project_id=project_id)
+            await self.project_repo.delete_project(user=user, project_id=ULID.from_str(project_id))
             return HTTPResponse(status=204)
 
         return "/projects/<project_id>", ["DELETE"], _delete
@@ -195,7 +196,7 @@ async def _patch(
             body_dict = body.model_dump(exclude_none=True)
 
             project_update = await self.project_repo.update_project(
-                user=user, project_id=project_id, etag=etag, payload=body_dict
+                user=user, project_id=ULID.from_str(project_id), etag=etag, payload=body_dict
             )
             if not isinstance(project_update, project_models.ProjectUpdate):
                 raise errors.ProgrammingError(
@@ -229,7 +230,7 @@ def get_all_members(self) -> BlueprintFactoryResponse:
         @authenticate(self.authenticator)
         @validate_path_project_id
         async def _get_all_members(_: Request, user: base_models.APIUser, project_id: str) -> JSONResponse:
-            members = await self.project_member_repo.get_members(user, project_id)
+            members = await self.project_member_repo.get_members(user, ULID.from_str(project_id))
 
             users = []
 
@@ -261,7 +262,7 @@ def update_members(self) -> BlueprintFactoryResponse:
         async def _update_members(request: Request, user: base_models.APIUser, project_id: str) -> HTTPResponse:
             body_dump = apispec.ProjectMemberListPatchRequest.model_validate(request.json)
             members = [Member(Role(i.role.value), i.id, project_id) for i in body_dump.root]
-            await self.project_member_repo.update_members(user, project_id, members)
+            await self.project_member_repo.update_members(user, ULID.from_str(project_id), members)
             return HTTPResponse(status=200)
 
         return "/projects/<project_id>/members", ["PATCH"], _update_members
@@ -275,7 +276,7 @@ def delete_member(self) -> BlueprintFactoryResponse:
         async def _delete_member(
             _: Request, user: base_models.APIUser, project_id: str, member_id: str
         ) -> HTTPResponse:
-            await self.project_member_repo.delete_members(user, project_id, [member_id])
+            await self.project_member_repo.delete_members(user, ULID.from_str(project_id), [member_id])
             return HTTPResponse(status=204)
 
         return "/projects/<project_id>/members/<member_id>", ["DELETE"], _delete_member

components/renku_data_services/project/db.py

@@ -76,7 +76,7 @@ async def get_projects(
 
     async def get_project(self, user: base_models.APIUser, project_id: ULID) -> models.Project:
         """Get one project from the database."""
-        authorized = await self.authz.has_permission(user, ResourceType.project, project_id, Scope.READ)
+        authorized = await self.authz.has_permission(user, ResourceType.project, str(project_id), Scope.READ)
         if not authorized:
             raise errors.MissingResourceError(
                 message=f"Project with id '{project_id}' does not exist or you do not have access to it."
@@ -113,7 +113,7 @@ async def get_project_by_namespace_slug(
             authorized = await self.authz.has_permission(
                 user=user,
                 resource_type=ResourceType.project,
-                resource_id=project_orm.id,
+                resource_id=str(project_orm.id),
                 scope=Scope.READ,
             )
             if not authorized:
@@ -212,7 +212,7 @@ async def update_project(
         if "namespace" in payload and payload["namespace"] != old_project.namespace:
             # NOTE: changing the namespace requires the user to be owner which means they should have DELETE permission
             required_scope = Scope.DELETE
-        authorized = await self.authz.has_permission(user, ResourceType.project, project_id, required_scope)
+        authorized = await self.authz.has_permission(user, ResourceType.project, str(project_id), required_scope)
         if not authorized:
             raise errors.MissingResourceError(
                 message=f"Project with id '{project_id_str}' does not exist or you do not have access to it."
@@ -273,7 +273,7 @@ async def delete_project(
         """Delete a project."""
         if not session:
             raise errors.ProgrammingError(message="A database session is required")
-        authorized = await self.authz.has_permission(user, ResourceType.project, project_id, Scope.DELETE)
+        authorized = await self.authz.has_permission(user, ResourceType.project, str(project_id), Scope.DELETE)
         if not authorized:
             raise errors.MissingResourceError(
                 message=f"Project with id '{project_id}' does not exist or you do not have access to it."
@@ -359,7 +359,7 @@ async def get_members(
         self, user: base_models.APIUser, project_id: ULID, *, session: AsyncSession | None = None
     ) -> list[Member]:
         """Get all members of a project."""
-        members = await self.authz.members(user, ResourceType.project, project_id)
+        members = await self.authz.members(user, ResourceType.project, str(project_id))
         members = [member for member in members if member.user_id and member.user_id != "*"]
         return members
 
@@ -391,7 +391,7 @@ async def update_members(
                 f"{requested_member_ids_set.difference(existing_member_ids)} cannot be found"
             )
 
-        output = await self.authz.upsert_project_members(user, ResourceType.project, project_id, members)
+        output = await self.authz.upsert_project_members(user, ResourceType.project, str(project_id), members)
         return output
 
     @with_db_transaction
@@ -404,5 +404,5 @@ async def delete_members(
         if len(user_ids) == 0:
             raise errors.ValidationError(message="Please request at least 1 member to be removed from the project")
 
-        members = await self.authz.remove_project_members(user, ResourceType.project, project_id, user_ids)
+        members = await self.authz.remove_project_members(user, ResourceType.project, str(project_id), user_ids)
         return members

components/renku_data_services/project/orm.py

@@ -54,7 +54,7 @@ class ProjectORM(BaseORM):
     def dump(self) -> models.Project:
         """Create a project model from the ProjectORM."""
         return models.Project(
-            id=self.id,
+            id=str(self.id),
             name=self.name,
             slug=self.slug.slug,
             namespace=self.slug.namespace.dump(),

components/renku_data_services/session/db.py

@@ -157,7 +157,9 @@ async def get_launcher(self, user: base_models.APIUser, launcher_id: ULID) -> mo
             launcher = res.one_or_none()
 
             authorized = (
-                await self.project_authz.has_permission(user, ResourceType.project, launcher.project_id, Scope.READ)
+                await self.project_authz.has_permission(
+                    user, ResourceType.project, str(launcher.project_id), Scope.READ
+                )
                 if launcher is not None
                 else False
             )
@@ -259,7 +261,7 @@ async def update_launcher(
             authorized = await self.project_authz.has_permission(
                 user,
                 ResourceType.project,
-                launcher.project_id,
+                str(launcher.project_id),
                 Scope.WRITE,
             )
             if not authorized:
@@ -336,7 +338,7 @@ async def delete_launcher(self, user: base_models.APIUser, launcher_id: ULID) ->
             authorized = await self.project_authz.has_permission(
                 user,
                 ResourceType.project,
-                launcher.project_id,
+                str(launcher.project_id),
                 Scope.WRITE,
             )
             if not authorized:

components/renku_data_services/session/orm.py

@@ -133,7 +133,7 @@ def load(cls, launcher: models.SessionLauncher) -> "SessionLauncherORM":
             description=launcher.description,
             environment_kind=launcher.environment_kind,
             container_image=launcher.container_image,
-            project_id=launcher.project_id,
+            project_id=ULID.from_str(launcher.project_id),
             environment_id=launcher.environment_id,
             resource_class_id=launcher.resource_class_id,
             default_url=launcher.default_url,
@@ -143,7 +143,7 @@ def dump(self) -> models.SessionLauncher:
         """Create a session launcher model from the SessionLauncherORM."""
         return models.SessionLauncher(
             id=self.id,
-            project_id=self.project_id,
+            project_id=str(self.project_id),
             name=self.name,
             created_by=models.Member(id=self.created_by_id),
             creation_date=self.creation_date,

components/renku_data_services/utils/sqlalchemy.py

@@ -22,4 +22,4 @@ def process_result_value(self, value: str | None, dialect: Dialect) -> ULID | No
         """Transform string from database into ULID."""
         if value is None:
             return None
-        return cast(ULID, ULID.from_str(value))
+        return cast(ULID, ULID.from_str(value))  # cast because mypy doesn't understand ULID type annotations