squashme: address comments

Author: olevski

components/renku_data_services/notebooks/core_sessions.py

@@ -469,13 +469,13 @@ def resources_patch_from_resource_class(resource_class: ResourceClass) -> Resour
     """Convert the resource class to a k8s resources spec."""
     gpu_name = GpuKind.NVIDIA.value + "/gpu"
     resources = resources_from_resource_class(resource_class)
-    requests: Mapping[str, Requests | RequestsStr | ResetType] | ResetType
-    limits: Mapping[str, Limits | LimitsStr | ResetType] | ResetType
+    requests: Mapping[str, Requests | RequestsStr | ResetType] | ResetType | None = None
+    limits: Mapping[str, Limits | LimitsStr | ResetType] | ResetType | None = None
     defaul_requests = {"memory": RESET, "cpu": RESET, gpu_name: RESET}
     default_limits = {"memory": RESET, "cpu": RESET, gpu_name: RESET}
-    if resources.requests:
+    if resources.requests is not None:
         requests = RESET if len(resources.requests.keys()) == 0 else {**defaul_requests, **resources.requests}
-    if resources.limits:
+    if resources.limits is not None:
         limits = RESET if len(resources.limits.keys()) == 0 else {**default_limits, **resources.limits}
     return ResourcesPatch(requests=requests, limits=limits)
 
@@ -1073,17 +1073,11 @@ async def patch_session(
                 )
             )
         rp = await rp_repo.get_resource_pool_from_class(user, body.resource_class_id)
-        try:
-            old_rp = await rp_repo.get_resource_pool_from_class(user, session.resource_class_id())
-        except (errors.MissingResourceError, errors.UnauthorizedError, errors.ForbiddenError):
-            old_rp = None
         rc = rp.get_resource_class(body.resource_class_id)
         if not rc:
             raise errors.MissingResourceError(
                 message=f"The resource class you requested with ID {body.resource_class_id} does not exist"
             )
-        if old_rp is not None and rp.cluster != old_rp.cluster:
-            raise errors.ValidationError(message="Changing resource pools with different clusters is not allowed.")
         if not patch.metadata:
             patch.metadata = AmaltheaSessionV1Alpha1MetadataPatch()
         # Patch the resource pool and class ID in the annotations

components/renku_data_services/notebooks/crs.py

@@ -154,7 +154,7 @@ def resource_class_id(self) -> int:
         return i
 
 
-class Culling(_ASCulling):
+class CullingDurationParsingMixin(BaseCRD):
     """Amalthea session culling configuration."""
 
     @field_serializer("*", mode="wrap")
@@ -171,6 +171,12 @@ def __deserialize_duration(cls, val: Any, handler: Any) -> Any:
         return handler(val)
 
 
+class Culling(_ASCulling, CullingDurationParsingMixin):
+    """Amalthea session culling configuration."""
+
+    pass
+
+
 class Requests(_Requests):
     """Resource requests of type integer."""
 
@@ -409,14 +415,14 @@ class AffinityPatch(BaseCRD):
     podAntiAffinity: PodAntiAffinity | ResetType | None = None
 
 
-class CullingPatch(Culling):
+class CullingPatch(CullingDurationParsingMixin):
     """Patch for the culling durations of a session."""
 
-    maxAge: timedelta | ResetType | None = None  # type:ignore[assignment]
-    maxFailedDuration: timedelta | ResetType | None = None  # type:ignore[assignment]
-    maxHibernatedDuration: timedelta | ResetType | None = None  # type:ignore[assignment]
-    maxIdleDuration: timedelta | ResetType | None = None  # type:ignore[assignment]
-    maxStartingDuration: timedelta | ResetType | None = None  # type:ignore[assignment]
+    maxAge: timedelta | ResetType | None = None
+    maxFailedDuration: timedelta | ResetType | None = None
+    maxHibernatedDuration: timedelta | ResetType | None = None
+    maxIdleDuration: timedelta | ResetType | None = None
+    maxStartingDuration: timedelta | ResetType | None = None
 
 
 class AmaltheaSessionV1Alpha1SpecPatch(BaseCRD):

test/bases/renku_data_services/data_api/test_notebooks.py

@@ -258,6 +258,10 @@ async def test_patch_server(
 
     assert res.status_code == expected_status_code, res.text
     if expected_status_code == 200 and patch == {"state": "hibernated"}:
+        # NOTE: The status of amalthea needs a few seconds to update and show up in the response
+        await asyncio.sleep(2)
+        _, res = await sanic_client.get(f"/api/data/sessions/{server_name}", headers=authenticated_user_headers)
+        assert res.status_code == 200
         assert res.json["status"]["state"] == "hibernated"
     if expected_status_code == 200 and "resource_class_id" in patch:
         assert res.json["resource_class_id"] == patch["resource_class_id"]