Merge pull request #3512 from SwissDataScienceCenter/release/fix-wff-plan-ids

fix(cli): fix wff plan ids

Author: Panaetius

.github/workflows/test_deploy.yml

@@ -405,7 +405,7 @@ jobs:
           LANG: en_US.UTF-8
           LC_ALL: en_US.UTF-8
           RENKU_REQUESTS_TIMEOUT_SECONDS: 120
-        run: pytest -v -m "not integration and not publish and not serial" tests/api
+        run: pytest -v -m "not integration and not publish and not serial and not service" tests/api
 
   test-macos-cli:
     runs-on: macos-latest
@@ -427,14 +427,14 @@ jobs:
           LANG: en_US.UTF-8
           LC_ALL: en_US.UTF-8
           RENKU_REQUESTS_TIMEOUT_SECONDS: 120
-        run: pytest -v -m "not integration and not publish and not serial" tests/cli
+        run: pytest -v -m "not integration and not publish and not serial and not service" tests/cli
       - name: Test with pytest (serial)
         env:
           POETRY_VIRTUALENVS_CREATE: false
           LANG: en_US.UTF-8
           LC_ALL: en_US.UTF-8
           RENKU_REQUESTS_TIMEOUT_SECONDS: 120
-        run: pytest -v -m "not integration and not publish and serial" tests/cli
+        run: pytest -v -m "not integration and not publish and serial and not service" tests/cli
 
   test-macos-core:
     runs-on: macos-latest
@@ -456,35 +456,13 @@ jobs:
           LANG: en_US.UTF-8
           LC_ALL: en_US.UTF-8
           RENKU_REQUESTS_TIMEOUT_SECONDS: 120
-        run: pytest -v -m "not integration and not publish and not serial" tests/core
+        run: pytest -v -m "not integration and not publish and not serial and not service" tests/core
       - name: Test with pytest (serial)
         env:
           LANG: en_US.UTF-8
           LC_ALL: en_US.UTF-8
           RENKU_REQUESTS_TIMEOUT_SECONDS: 120
-        run: pytest -v -m "not integration and not publish and serial" tests/core
-
-  test-macos-service:
-    runs-on: macos-latest
-    needs: [ set-matrix ]
-    strategy:
-      max-parallel: 3
-      matrix: ${{ fromJson(needs.set-matrix.outputs.matrix) }}
-    steps:
-      - uses: actions/[email protected]
-        with:
-          fetch-depth: 0
-      - name: Install dependencies
-        uses: ./.github/actions/install-macos
-        with:
-          python-version: ${{ matrix.python-version }}
-      - name: Test with pytest
-        env:
-          POETRY_VIRTUALENVS_CREATE: false
-          LANG: en_US.UTF-8
-          LC_ALL: en_US.UTF-8
-          RENKU_REQUESTS_TIMEOUT_SECONDS: 120
-        run: pytest -v -m "not integration and not publish" tests/service
+        run: pytest -v -m "not integration and not publish and serial and not service" tests/core
 
   test-macos-integration:
     runs-on: macos-latest
@@ -518,7 +496,7 @@ jobs:
           CLOUD_STORAGE_AZURE_KEY: ${{ secrets.CLOUD_STORAGE_AZURE_KEY }}
           CLOUD_STORAGE_S3_ACCESS_KEY_ID: ${{ secrets.CLOUD_STORAGE_S3_ACCESS_KEY_ID }}
           CLOUD_STORAGE_S3_SECRET_ACCESS_KEY: ${{ secrets.CLOUD_STORAGE_S3_SECRET_ACCESS_KEY }}
-        run: pytest -m "integration and not serial" -v
+        run: pytest -m "integration and not serial and not service" -v
       - name: Start Redis
         uses: supercharge/[email protected]
       - name: Test with pytest (serial)
@@ -531,7 +509,7 @@ jobs:
           ZENODO_ACCESS_TOKEN: ${{ secrets.ZENODO_ACCESS_TOKEN }}
           OLOS_ACCESS_TOKEN: ${{ secrets.OLOS_ACCESS_TOKEN }}
           RENKU_REQUESTS_TIMEOUT_SECONDS: 120
-        run: pytest -m "integration and serial" -v
+        run: pytest -m "integration and serial and not service" -v
 
   publish-pypi:
     runs-on: ubuntu-latest
@@ -573,7 +551,6 @@ jobs:
         test-linux-service,
         test-macos-cli,
         test-macos-core,
-        test-macos-service,
       ]
     steps:
       - uses: actions/[email protected]
@@ -602,7 +579,6 @@ jobs:
         test-linux-service,
         test-macos-cli,
         test-macos-core,
-        test-macos-service,
       ]
     if: startsWith(github.ref, 'refs/tags/')
     steps:

CHANGES.rst

@@ -30,6 +30,8 @@ Bug Fixes
 -  **cli:** fix special paths in workflow files and bump `toil` / `cwltool`
    (`#3489 <https://github.com/SwissDataScienceCenter/renku-python/issues/3489>`__)
    (`28086cf <https://github.com/SwissDataScienceCenter/renku-python/commit/28086cf1361c86109c8e0e1c59c5704a5a663f30>`__)
+- **cli:**: fix wrong plan ids in plans coming from workflow files
+   (`#3511 <https://github.com/SwissDataScienceCenter/renku-python/pull/3511>`__)
 -  **service:** fix working with branches
    (`#3472 <https://github.com/SwissDataScienceCenter/renku-python/issues/3472>`__)
    (`0eaf204 <https://github.com/SwissDataScienceCenter/renku-python/commit/0eaf204365d38bbf82bd2d0df357abbf61c18548>`__)

renku/command/schema/workflow_file.py

@@ -15,6 +15,9 @@
 # limitations under the License.
 """Represent workflow file run templates."""
 
+from itertools import chain
+from typing import List, Union
+
 import marshmallow
 
 from renku.command.schema.calamus import fields, prov, renku, schema
@@ -33,6 +36,26 @@ class Meta:
         model = WorkflowFilePlan
         unknown = marshmallow.EXCLUDE
 
+    @marshmallow.pre_dump(pass_many=True)
+    def fix_ids(self, objs: Union[WorkflowFilePlan, List[WorkflowFilePlan]], many, **kwargs):
+        """Renku up to 2.4.1 had a bug that created wrong ids for workflow file entities, this fixes those on export."""
+
+        def _replace_id(obj):
+            obj.unfreeze()
+            obj.id = obj.id.replace("//plans/", "/")
+
+            for child in chain(obj.inputs, obj.outputs, obj.parameters):
+                child.id = child.id.replace("//plans/", "/")
+            obj.freeze()
+
+        if many:
+            for obj in objs:
+                _replace_id(obj)
+            return objs
+
+        _replace_id(objs)
+        return objs
+
 
 class WorkflowFileCompositePlanSchema(CompositePlanSchema):
     """Plan schema."""
@@ -46,3 +69,20 @@ class Meta:
 
     path = fields.String(prov.atLocation)
     plans = fields.Nested(renku.hasSubprocess, WorkflowFilePlanSchema, many=True)
+
+    @marshmallow.pre_dump(pass_many=True)
+    def fix_ids(self, objs, many, **kwargs):
+        """Renku up to 2.4.1 had a bug that created wrong ids for workflow file entities, this fixes those on export."""
+
+        def _replace_id(obj):
+            obj.unfreeze()
+            obj.id = obj.id.replace("//plans/", "/")
+            obj.freeze()
+
+        if many:
+            for obj in objs:
+                _replace_id(obj)
+            return objs
+
+        _replace_id(objs)
+        return objs

renku/domain_model/workflow/workflow_file.py

@@ -37,19 +37,30 @@ def __init__(self, *, path: Union[Path, str], **kwargs):
         self.path: str = str(path)
 
     @staticmethod
-    def generate_id(path: Optional[Union[Path, str]] = None, sequence: Optional[int] = None, **_) -> str:
+    def generate_id(
+        path: Optional[Union[Path, str]] = None, sequence: Optional[int] = None, uuid_only: bool = False, **_
+    ) -> str:
         """Generate an identifier for Plan."""
         assert path, "Path is needed to generate id for WorkflowFileCompositePlan"
 
         # NOTE: Workflow file's root composite plan's ID is generated only based on the file's path. The ID might be
         # changed later if the plan is a derivative
         key = f"{path}" if sequence is None else f"{path}::{sequence}"
         key_bytes = key.encode("utf-8")
-        return CompositePlan.generate_id(uuid=hashlib.md5(key_bytes).hexdigest()[:32])  # nosec
+
+        uuid = hashlib.md5(key_bytes).hexdigest()[:32]  # nosec
+
+        if uuid_only:
+            return uuid
+        return CompositePlan.generate_id(uuid=uuid)
 
     def assign_new_id(self, *, sequence: Optional[int] = None, **_) -> str:
         """Assign a new UUID or a deterministic."""
-        new_id = uuid.uuid4().hex if sequence is None else self.generate_id(path=self.path, sequence=sequence)
+        new_id = (
+            uuid.uuid4().hex
+            if sequence is None
+            else self.generate_id(path=self.path, sequence=sequence, uuid_only=True)
+        )
         return super().assign_new_id(uuid=new_id)
 
     def is_equal_to(self, other: WorkflowFileCompositePlan) -> bool:
@@ -67,15 +78,22 @@ def __init__(self, *, path: Union[Path, str], **kwargs):
 
     @staticmethod
     def generate_id(
-        path: Optional[Union[Path, str]] = None, name: Optional[str] = None, sequence: Optional[int] = None, **_
+        path: Optional[Union[Path, str]] = None,
+        name: Optional[str] = None,
+        sequence: Optional[int] = None,
+        uuid_only: bool = False,
+        **_,
     ) -> str:
         """Generate an identifier for Plan."""
         assert path, "Path is needed to generate id for WorkflowFilePlan"
         assert name, "Name is needed to generate id for WorkflowFilePlan"
 
         key = f"{path}::{name}" if sequence is None else f"{path}::{name}::{sequence}"
         key_bytes = key.encode("utf-8")
-        return Plan.generate_id(uuid=hashlib.md5(key_bytes).hexdigest()[:32])  # nosec
+        uuid = hashlib.md5(key_bytes).hexdigest()[:32]  # nosec
+        if uuid_only:
+            return uuid
+        return Plan.generate_id(uuid=uuid)
 
     @staticmethod
     def validate_name(name: str):
@@ -92,7 +110,7 @@ def assign_new_id(self, *, sequence: Optional[int] = None, **_) -> str:
         new_id = (
             uuid.uuid4().hex
             if sequence is None
-            else self.generate_id(path=self.path, name=self.name, sequence=sequence)
+            else self.generate_id(path=self.path, name=self.name, sequence=sequence, uuid_only=True)
         )
         return super().assign_new_id(uuid=new_id)