Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix setting task run mode in Edit Runtime #65

Open
wants to merge 2 commits into
base: feature.skip_mode
Choose a base branch
from
+33 −6
Open

Fix setting task run mode in Edit Runtime #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6391b47
Fix run mode printed in scheduler log
MetRonnie Nov 12, 2024
d3c2b63
Fix setting task run mode in Edit Runtime
MetRonnie Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions cylc/flow/network/resolvers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@
from cylc.flow.id import Tokens
from cylc.flow.network.schema import (
DEF_TYPES,
RUNTIME_FIELD_TO_CFG_MAP,
NodesEdges,
PROXY_NODES,
SUB_RESOLVERS,
runtime_schema_to_cfg,
sort_elements,
)

Expand Down Expand Up @@ -791,10 +791,8 @@ def broadcast(
# Convert schema field names to workflow config setting names if
# applicable:
for i, dict_ in enumerate(settings):
settings[i] = {
RUNTIME_FIELD_TO_CFG_MAP.get(key, key): value
for key, value in dict_.items()
}
settings[i] = runtime_schema_to_cfg(dict_)

if mode == 'put_broadcast':
return self.schd.task_events_mgr.broadcast_mgr.put_broadcast(
cycle_points, namespaces, settings)
Expand Down
14 changes: 14 additions & 0 deletions cylc/flow/network/schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,20 @@ class Meta:
"""Map GQL Runtime fields' names to workflow config setting names."""


def runtime_schema_to_cfg(runtime: dict) -> dict:
"""Covert GQL Runtime field names to workflow config setting names and
perform any necessary processing on the values."""
# We have to manually lowercase the run_mode field because we don't define
# a proper schema for BroadcastSetting (it's just GenericScalar) so
# Graphene has no way to know that it should be a TaskRunMode enum.
Comment on lines +866 to +868
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FAO @oliver-sanders

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the graphene.Enum back to front per-chance?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the Enum works correctly as far as creating the dropdown and sending the result back

image

This is because the Edit Runtime form is built from the result of querying the runtime of the task proxy:

class Runtime(ObjectType):
    ...
    run_mode = TaskRunMode(default_value=TaskRunMode.Live.name)

The problem is that in the mutation it sends back, there is no longer any association with this Enum, all you get is List(BroadcastSetting) which is the same as List(GenericScalar)

class Broadcast(Mutation):
    ...
    class Arguments:
        ...
        settings = graphene.List(
            BroadcastSetting,
            description=sstrip('''
                The `[runtime]` configuration to override with this broadcast
                (e.g. `script = true` or `[environment]ANSWER=42`).
            '''),

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(sorry, was slow on the uptake there)

return {
RUNTIME_FIELD_TO_CFG_MAP.get(key, key): (
value.lower() if key == 'run_mode' else value
)
for key, value in runtime.items()
}


class Job(ObjectType):
class Meta:
description = "Jobs."
Expand Down
2 changes: 1 addition & 1 deletion cylc/flow/scheduler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ def log_start(self) -> None:
# Note that the following lines must be present at the top of
# the workflow log file for use in reference test runs.
LOG.info(
f'Run mode: {self.get_run_mode()}',
f'Run mode: {self.get_run_mode().value}',
extra=RotatingLogFileHandler.header_extra
)
LOG.info(
Expand Down
15 changes: 15 additions & 0 deletions tests/unit/network/test_schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
RUNTIME_FIELD_TO_CFG_MAP,
Mutations,
Runtime,
runtime_schema_to_cfg,
sort_elements,
SortArgs,
)
Expand Down Expand Up @@ -105,6 +106,20 @@ def test_runtime_field_to_cfg_map(field_name: str):
assert WORKFLOW_SPEC.get('runtime', '__MANY__', cfg_name)


@pytest.mark.parametrize('runtime_dict,expected', [
pytest.param(
{'run_mode': 'Skip'}, {'run mode': 'skip'}, id='edit-runtime'
),
pytest.param(
{'run mode': 'skip'}, {'run mode': 'skip'}, id='broadcast'
),
])
def test_runtime_schema_to_cfg(runtime_dict, expected):
"""Test this function can handle Edit Runtime submitted values as well
as normal broadcast values."""
assert runtime_schema_to_cfg(runtime_dict) == expected


@pytest.mark.parametrize('mutation', (
pytest.param(attr, id=name)
for name, attr in Mutations.__dict__.items()
Expand Down
Loading