diff --git a/.github/workflows/test_deploy.yml b/.github/workflows/test_deploy.yml index 1a975ce513..929f45632f 100644 --- a/.github/workflows/test_deploy.yml +++ b/.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/checkout@v3.5.2 - 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/redis-github-action@1.5.0 - 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/checkout@v3.5.2 @@ -602,7 +579,6 @@ jobs: test-linux-service, test-macos-cli, test-macos-core, - test-macos-service, ] if: startsWith(github.ref, 'refs/tags/') steps: diff --git a/CHANGES.rst b/CHANGES.rst index d8106745dd..beef56d468 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -30,6 +30,8 @@ Bug Fixes - **cli:** fix special paths in workflow files and bump `toil` / `cwltool` (`#3489 `__) (`28086cf `__) +- **cli:**: fix wrong plan ids in plans coming from workflow files + (`#3511 `__) - **service:** fix working with branches (`#3472 `__) (`0eaf204 `__) diff --git a/renku/command/schema/workflow_file.py b/renku/command/schema/workflow_file.py index 73b985031c..c44ab43024 100644 --- a/renku/command/schema/workflow_file.py +++ b/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 diff --git a/renku/domain_model/workflow/workflow_file.py b/renku/domain_model/workflow/workflow_file.py index 2f5633ed6d..5058737f86 100644 --- a/renku/domain_model/workflow/workflow_file.py +++ b/renku/domain_model/workflow/workflow_file.py @@ -37,7 +37,9 @@ 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" @@ -45,11 +47,20 @@ def generate_id(path: Optional[Union[Path, str]] = None, sequence: Optional[int] # 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,7 +78,11 @@ 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" @@ -75,7 +90,10 @@ def generate_id( 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)