Skip to content

Commit 134f943

Browse files
PanaetiusPanaetius
authored
Merge pull request #3512 from SwissDataScienceCenter/release/fix-wff-plan-ids
fix(cli): fix wff plan ids
2 parents fc50767 + baa248d commit 134f943

4 files changed

Lines changed: 73 additions & 37 deletions

File tree

‎.github/workflows/test_deploy.yml‎

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ jobs:
405405
LANG: en_US.UTF-8
406406
LC_ALL: en_US.UTF-8
407407
RENKU_REQUESTS_TIMEOUT_SECONDS: 120
408-
run: pytest -v -m "not integration and not publish and not serial" tests/api
408+
run: pytest -v -m "not integration and not publish and not serial and not service" tests/api
409409

410410
test-macos-cli:
411411
runs-on: macos-latest
@@ -427,14 +427,14 @@ jobs:
427427
LANG: en_US.UTF-8
428428
LC_ALL: en_US.UTF-8
429429
RENKU_REQUESTS_TIMEOUT_SECONDS: 120
430-
run: pytest -v -m "not integration and not publish and not serial" tests/cli
430+
run: pytest -v -m "not integration and not publish and not serial and not service" tests/cli
431431
- name: Test with pytest (serial)
432432
env:
433433
POETRY_VIRTUALENVS_CREATE: false
434434
LANG: en_US.UTF-8
435435
LC_ALL: en_US.UTF-8
436436
RENKU_REQUESTS_TIMEOUT_SECONDS: 120
437-
run: pytest -v -m "not integration and not publish and serial" tests/cli
437+
run: pytest -v -m "not integration and not publish and serial and not service" tests/cli
438438

439439
test-macos-core:
440440
runs-on: macos-latest
@@ -456,35 +456,13 @@ jobs:
456456
LANG: en_US.UTF-8
457457
LC_ALL: en_US.UTF-8
458458
RENKU_REQUESTS_TIMEOUT_SECONDS: 120
459-
run: pytest -v -m "not integration and not publish and not serial" tests/core
459+
run: pytest -v -m "not integration and not publish and not serial and not service" tests/core
460460
- name: Test with pytest (serial)
461461
env:
462462
LANG: en_US.UTF-8
463463
LC_ALL: en_US.UTF-8
464464
RENKU_REQUESTS_TIMEOUT_SECONDS: 120
465-
run: pytest -v -m "not integration and not publish and serial" tests/core
466-
467-
test-macos-service:
468-
runs-on: macos-latest
469-
needs: [ set-matrix ]
470-
strategy:
471-
max-parallel: 3
472-
matrix: ${{ fromJson(needs.set-matrix.outputs.matrix) }}
473-
steps:
474-
- uses: actions/checkout@v3.5.2
475-
with:
476-
fetch-depth: 0
477-
- name: Install dependencies
478-
uses: ./.github/actions/install-macos
479-
with:
480-
python-version: ${{ matrix.python-version }}
481-
- name: Test with pytest
482-
env:
483-
POETRY_VIRTUALENVS_CREATE: false
484-
LANG: en_US.UTF-8
485-
LC_ALL: en_US.UTF-8
486-
RENKU_REQUESTS_TIMEOUT_SECONDS: 120
487-
run: pytest -v -m "not integration and not publish" tests/service
465+
run: pytest -v -m "not integration and not publish and serial and not service" tests/core
488466

489467
test-macos-integration:
490468
runs-on: macos-latest
@@ -518,7 +496,7 @@ jobs:
518496
CLOUD_STORAGE_AZURE_KEY: ${{ secrets.CLOUD_STORAGE_AZURE_KEY }}
519497
CLOUD_STORAGE_S3_ACCESS_KEY_ID: ${{ secrets.CLOUD_STORAGE_S3_ACCESS_KEY_ID }}
520498
CLOUD_STORAGE_S3_SECRET_ACCESS_KEY: ${{ secrets.CLOUD_STORAGE_S3_SECRET_ACCESS_KEY }}
521-
run: pytest -m "integration and not serial" -v
499+
run: pytest -m "integration and not serial and not service" -v
522500
- name: Start Redis
523501
uses: supercharge/redis-github-action@1.5.0
524502
- name: Test with pytest (serial)
@@ -531,7 +509,7 @@ jobs:
531509
ZENODO_ACCESS_TOKEN: ${{ secrets.ZENODO_ACCESS_TOKEN }}
532510
OLOS_ACCESS_TOKEN: ${{ secrets.OLOS_ACCESS_TOKEN }}
533511
RENKU_REQUESTS_TIMEOUT_SECONDS: 120
534-
run: pytest -m "integration and serial" -v
512+
run: pytest -m "integration and serial and not service" -v
535513

536514
publish-pypi:
537515
runs-on: ubuntu-latest
@@ -573,7 +551,6 @@ jobs:
573551
test-linux-service,
574552
test-macos-cli,
575553
test-macos-core,
576-
test-macos-service,
577554
]
578555
steps:
579556
- uses: actions/checkout@v3.5.2
@@ -602,7 +579,6 @@ jobs:
602579
test-linux-service,
603580
test-macos-cli,
604581
test-macos-core,
605-
test-macos-service,
606582
]
607583
if: startsWith(github.ref, 'refs/tags/')
608584
steps:

‎CHANGES.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ Bug Fixes
3030
- **cli:** fix special paths in workflow files and bump `toil` / `cwltool`
3131
(`#3489 <https://github.com/SwissDataScienceCenter/renku-python/issues/3489>`__)
3232
(`28086cf <https://github.com/SwissDataScienceCenter/renku-python/commit/28086cf1361c86109c8e0e1c59c5704a5a663f30>`__)
33+
- **cli:**: fix wrong plan ids in plans coming from workflow files
34+
(`#3511 <https://github.com/SwissDataScienceCenter/renku-python/pull/3511>`__)
3335
- **service:** fix working with branches
3436
(`#3472 <https://github.com/SwissDataScienceCenter/renku-python/issues/3472>`__)
3537
(`0eaf204 <https://github.com/SwissDataScienceCenter/renku-python/commit/0eaf204365d38bbf82bd2d0df357abbf61c18548>`__)

‎renku/command/schema/workflow_file.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
# limitations under the License.
1616
"""Represent workflow file run templates."""
1717

18+
from itertools import chain
19+
from typing import List, Union
20+
1821
import marshmallow
1922

2023
from renku.command.schema.calamus import fields, prov, renku, schema
@@ -33,6 +36,26 @@ class Meta:
3336
model = WorkflowFilePlan
3437
unknown = marshmallow.EXCLUDE
3538

39+
@marshmallow.pre_dump(pass_many=True)
40+
def fix_ids(self, objs: Union[WorkflowFilePlan, List[WorkflowFilePlan]], many, **kwargs):
41+
"""Renku up to 2.4.1 had a bug that created wrong ids for workflow file entities, this fixes those on export."""
42+
43+
def _replace_id(obj):
44+
obj.unfreeze()
45+
obj.id = obj.id.replace("//plans/", "/")
46+
47+
for child in chain(obj.inputs, obj.outputs, obj.parameters):
48+
child.id = child.id.replace("//plans/", "/")
49+
obj.freeze()
50+
51+
if many:
52+
for obj in objs:
53+
_replace_id(obj)
54+
return objs
55+
56+
_replace_id(objs)
57+
return objs
58+
3659

3760
class WorkflowFileCompositePlanSchema(CompositePlanSchema):
3861
"""Plan schema."""
@@ -46,3 +69,20 @@ class Meta:
4669

4770
path = fields.String(prov.atLocation)
4871
plans = fields.Nested(renku.hasSubprocess, WorkflowFilePlanSchema, many=True)
72+
73+
@marshmallow.pre_dump(pass_many=True)
74+
def fix_ids(self, objs, many, **kwargs):
75+
"""Renku up to 2.4.1 had a bug that created wrong ids for workflow file entities, this fixes those on export."""
76+
77+
def _replace_id(obj):
78+
obj.unfreeze()
79+
obj.id = obj.id.replace("//plans/", "/")
80+
obj.freeze()
81+
82+
if many:
83+
for obj in objs:
84+
_replace_id(obj)
85+
return objs
86+
87+
_replace_id(objs)
88+
return objs

‎renku/domain_model/workflow/workflow_file.py‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,30 @@ def __init__(self, *, path: Union[Path, str], **kwargs):
3737
self.path: str = str(path)
3838

3939
@staticmethod
40-
def generate_id(path: Optional[Union[Path, str]] = None, sequence: Optional[int] = None, **_) -> str:
40+
def generate_id(
41+
path: Optional[Union[Path, str]] = None, sequence: Optional[int] = None, uuid_only: bool = False, **_
42+
) -> str:
4143
"""Generate an identifier for Plan."""
4244
assert path, "Path is needed to generate id for WorkflowFileCompositePlan"
4345

4446
# NOTE: Workflow file's root composite plan's ID is generated only based on the file's path. The ID might be
4547
# changed later if the plan is a derivative
4648
key = f"{path}" if sequence is None else f"{path}::{sequence}"
4749
key_bytes = key.encode("utf-8")
48-
return CompositePlan.generate_id(uuid=hashlib.md5(key_bytes).hexdigest()[:32]) # nosec
50+
51+
uuid = hashlib.md5(key_bytes).hexdigest()[:32] # nosec
52+
53+
if uuid_only:
54+
return uuid
55+
return CompositePlan.generate_id(uuid=uuid)
4956

5057
def assign_new_id(self, *, sequence: Optional[int] = None, **_) -> str:
5158
"""Assign a new UUID or a deterministic."""
52-
new_id = uuid.uuid4().hex if sequence is None else self.generate_id(path=self.path, sequence=sequence)
59+
new_id = (
60+
uuid.uuid4().hex
61+
if sequence is None
62+
else self.generate_id(path=self.path, sequence=sequence, uuid_only=True)
63+
)
5364
return super().assign_new_id(uuid=new_id)
5465

5566
def is_equal_to(self, other: WorkflowFileCompositePlan) -> bool:
@@ -67,15 +78,22 @@ def __init__(self, *, path: Union[Path, str], **kwargs):
6778

6879
@staticmethod
6980
def generate_id(
70-
path: Optional[Union[Path, str]] = None, name: Optional[str] = None, sequence: Optional[int] = None, **_
81+
path: Optional[Union[Path, str]] = None,
82+
name: Optional[str] = None,
83+
sequence: Optional[int] = None,
84+
uuid_only: bool = False,
85+
**_,
7186
) -> str:
7287
"""Generate an identifier for Plan."""
7388
assert path, "Path is needed to generate id for WorkflowFilePlan"
7489
assert name, "Name is needed to generate id for WorkflowFilePlan"
7590

7691
key = f"{path}::{name}" if sequence is None else f"{path}::{name}::{sequence}"
7792
key_bytes = key.encode("utf-8")
78-
return Plan.generate_id(uuid=hashlib.md5(key_bytes).hexdigest()[:32]) # nosec
93+
uuid = hashlib.md5(key_bytes).hexdigest()[:32] # nosec
94+
if uuid_only:
95+
return uuid
96+
return Plan.generate_id(uuid=uuid)
7997

8098
@staticmethod
8199
def validate_name(name: str):
@@ -92,7 +110,7 @@ def assign_new_id(self, *, sequence: Optional[int] = None, **_) -> str:
92110
new_id = (
93111
uuid.uuid4().hex
94112
if sequence is None
95-
else self.generate_id(path=self.path, name=self.name, sequence=sequence)
113+
else self.generate_id(path=self.path, name=self.name, sequence=sequence, uuid_only=True)
96114
)
97115
return super().assign_new_id(uuid=new_id)
98116

0 commit comments

Comments
 (0)