fractal-analytics-platform / fractal-web

Web client for Fractal
https://fractal-analytics-platform.github.io/fractal-web/
BSD 3-Clause "New" or "Revised" License
6 stars 0 forks source link

Deprecate support for legacy tasks in V2 workflows #545

Closed tcompa closed 1 day ago

tcompa commented 2 weeks ago

This cannot be merged&released until we verify that production instances do not include any such task. Meanwhile, we are preparing the fractal-server update in https://github.com/fractal-analytics-platform/fractal-server/pull/1721. It is still partly in-progress (e.g. it doesn't work with sqlite, and we need to update tests and double-check the update path), but the corresponding fractal-web update can already be developed.

Here is, for instance, the diff of the WorkflowTaskV2 model

diff --git a/fractal_server/app/models/v2/workflowtask.py b/fractal_server/app/models/v2/workflowtask.py
index 75e0f28b2..32f64215a 100644
--- a/fractal_server/app/models/v2/workflowtask.py
+++ b/fractal_server/app/models/v2/workflowtask.py
@@ -8,7 +8,6 @@ from sqlmodel import Field
 from sqlmodel import Relationship
 from sqlmodel import SQLModel

-from ..v1.task import Task
 from .task import TaskV2

@@ -37,13 +36,6 @@ class WorkflowTaskV2(SQLModel, table=True):
     )

     # Task
-    is_legacy_task: bool
     task_type: str
-    task_id: Optional[int] = Field(foreign_key="taskv2.id")
-    task: Optional[TaskV2] = Relationship(
-        sa_relationship_kwargs=dict(lazy="selectin")
-    )
-    task_legacy_id: Optional[int] = Field(foreign_key="task.id")
-    task_legacy: Optional[Task] = Relationship(
-        sa_relationship_kwargs=dict(lazy="selectin")
-    )
+    task_id: int = Field(foreign_key="taskv2.id")
+    task: TaskV2 = Relationship(sa_relationship_kwargs=dict(lazy="selectin"))

and here are the diffs for some API schemas

diff --git a/fractal_server/app/schemas/v2/dumps.py b/fractal_server/app/schemas/v2/dumps.py
index 8aa625ed5..1b733b064 100644
--- a/fractal_server/app/schemas/v2/dumps.py
+++ b/fractal_server/app/schemas/v2/dumps.py
@@ -12,9 +12,7 @@ from typing import Optional

 from pydantic import BaseModel
 from pydantic import Extra
-from pydantic import root_validator

-from fractal_server.app.schemas.v1.dumps import TaskDumpV1
 from fractal_server.images import Filters

@@ -45,29 +43,10 @@ class WorkflowTaskDumpV2(BaseModel):
     workflow_id: int
     order: Optional[int]

-    is_legacy_task: bool
-
     input_filters: Filters

-    task_id: Optional[int]
-    task: Optional[TaskDumpV2]
-    task_legacy_id: Optional[int]
-    task_legacy: Optional[TaskDumpV1]
-
-    # Validators
-    @root_validator
-    def task_v1_or_v2(cls, values):
-        v1 = values.get("task_legacy_id")
-        v2 = values.get("task_id")
-        if ((v1 is not None) and (v2 is not None)) or (
-            (v1 is None) and (v2 is None)
-        ):
-            message = "both" if (v1 and v2) else "none"
-            raise ValueError(
-                "One and only one must be provided between "
-                f"'task_legacy_id' and 'task_id' (you provided {message})"
-            )
-        return values
+    task_id: int
+    task: TaskDumpV2

 class WorkflowDumpV2(BaseModel, extra=Extra.forbid):
diff --git a/fractal_server/app/schemas/v2/workflowtask.py b/fractal_server/app/schemas/v2/workflowtask.py
index 9303dca17..1edd35e2d 100644
--- a/fractal_server/app/schemas/v2/workflowtask.py
+++ b/fractal_server/app/schemas/v2/workflowtask.py
@@ -5,16 +5,12 @@ from typing import Optional
 from pydantic import BaseModel
 from pydantic import Extra
 from pydantic import Field
-from pydantic import root_validator
 from pydantic import validator

 from .._validators import valdictkeys
 from .._validators import valint
-from ..v1.task import TaskExportV1
-from ..v1.task import TaskImportV1
 from .task import TaskExportV2
 from .task import TaskImportV2
-from .task import TaskLegacyReadV2
 from .task import TaskReadV2
 from fractal_server.images import Filters

@@ -49,8 +45,6 @@ class WorkflowTaskCreateV2(BaseModel, extra=Extra.forbid):
     order: Optional[int]
     input_filters: Filters = Field(default_factory=Filters)

-    is_legacy_task: bool = False
-
     # Validators
     _meta_non_parallel = validator("meta_non_parallel", allow_reuse=True)(
         valdictkeys("meta_non_parallel")
@@ -88,18 +82,6 @@ class WorkflowTaskCreateV2(BaseModel, extra=Extra.forbid):
             )
         return value

-    @root_validator
-    def validate_legacy_task(cls, values):
-        if values["is_legacy_task"] and (
-            values.get("meta_non_parallel") is not None
-            or values.get("args_non_parallel") is not None
-        ):
-            raise ValueError(
-                "If Task is legacy, 'args_non_parallel' and 'meta_non_parallel"
-                "must be None"
-            )
-        return values
-

 class WorkflowTaskReadV2(BaseModel):

@@ -115,12 +97,9 @@ class WorkflowTaskReadV2(BaseModel):

     input_filters: Filters

-    is_legacy_task: bool
     task_type: str
-    task_id: Optional[int]
-    task: Optional[TaskReadV2]
-    task_legacy_id: Optional[int]
-    task_legacy: Optional[TaskLegacyReadV2]
+    task_id: int
+    task: TaskReadV2

 class WorkflowTaskUpdateV2(BaseModel):
@@ -177,9 +156,7 @@ class WorkflowTaskImportV2(BaseModel):

     input_filters: Optional[Filters] = None

-    is_legacy_task: bool = False
-    task: Optional[TaskImportV2] = None
-    task_legacy: Optional[TaskImportV1] = None
+    task: TaskImportV2

     _meta_non_parallel = validator("meta_non_parallel", allow_reuse=True)(
         valdictkeys("meta_non_parallel")
@@ -203,6 +180,4 @@ class WorkflowTaskExportV2(BaseModel):
     args_parallel: Optional[dict[str, Any]] = None
     input_filters: Filters = Field(default_factory=Filters)

-    is_legacy_task: bool = False
-    task: Optional[TaskExportV2]
-    task_legacy: Optional[TaskExportV1]
tcompa commented 6 days ago

This deprecation will take place in fractal-server 2.5.0, which will have to come together with some future fractal-web release.

tcompa commented 4 days ago

Fractal-server 2.5.0a0 is now available, so that this feature can be developed/tested.