Skip to content

Commit

Permalink
broadcast: report invalid settings
Browse files Browse the repository at this point in the history
* We already report invalid cycle points and namespaces.
* However, we have been relying on client-side validation for settings
  (which doesn't apply to the GraphQL mutation).
* This also raises the potential for inter-version compatibility issues
  going unreported.
* This commit explicitly handles invalid settings in the same way as
  invalid cycle points and namespaces so that they are reported back to
  the user.
* Closes the issue part of cylc#6429.
  • Loading branch information
oliver-sanders committed Jan 24, 2025
1 parent a49b4cd commit 36ba052
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 33 deletions.
16 changes: 12 additions & 4 deletions cylc/flow/broadcast_mgr.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ def put_broadcast(
bad_options is as described in the docstring for self.clear().
"""
modified_settings = []
bad_settings = []
bad_point_strings = []
bad_namespaces = []

Expand All @@ -284,10 +285,15 @@ def put_broadcast(
# Coerce setting to cylc runtime object,
# i.e. str to DurationFloat.
coerced_setting = deepcopy(setting)
BroadcastConfigValidator().validate(
coerced_setting,
SPEC['runtime']['__MANY__'],
)
try:
BroadcastConfigValidator().validate(
coerced_setting,
SPEC['runtime']['__MANY__'],
)
except Exception as exc:
LOG.error(exc)
bad_settings.append(setting)
continue

for point_string in point_strings or []:
# Standardise the point and check its validity.
Expand Down Expand Up @@ -324,6 +330,8 @@ def put_broadcast(
LOG.info(get_broadcast_change_report(modified_settings))

bad_options = {}
if bad_settings:
bad_options["settings"] = bad_settings
if bad_point_strings:
bad_options["point_strings"] = bad_point_strings
if bad_namespaces:
Expand Down
41 changes: 23 additions & 18 deletions cylc/flow/network/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -787,24 +787,29 @@ def broadcast(
cutoff: Any = None
):
"""Put or clear broadcasts."""
if settings is not None:
# Convert schema field names to workflow config setting names if
# applicable:
for i, dict_ in enumerate(settings):
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)
if mode == 'clear_broadcast':
return self.schd.task_events_mgr.broadcast_mgr.clear_broadcast(
point_strings=cycle_points,
namespaces=namespaces,
cancel_settings=settings)
if mode == 'expire_broadcast':
return self.schd.task_events_mgr.broadcast_mgr.expire_broadcast(
cutoff)
raise ValueError('Unsupported broadcast mode')
try:
if settings is not None:
# Convert schema field names to workflow config setting names if
# applicable:
for i, dict_ in enumerate(settings):
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)
if mode == 'clear_broadcast':
return self.schd.task_events_mgr.broadcast_mgr.clear_broadcast(
point_strings=cycle_points,
namespaces=namespaces,
cancel_settings=settings)
if mode == 'expire_broadcast':
return self.schd.task_events_mgr.broadcast_mgr.expire_broadcast(
cutoff)
raise ValueError('Unsupported broadcast mode')
except Exception as exc:
LOG.error(exc)
return {'error': {'message': str(exc)}}


def put_ext_trigger(
self,
Expand Down
11 changes: 0 additions & 11 deletions cylc/flow/network/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -1778,17 +1778,6 @@ class Arguments:
)
)

# TODO: work out how to implement this feature, it needs to be
# handled client-side which makes it slightly awkward in
# api-on-the-fly land

# files = graphene.List(
# String,
# description=sstrip('''
# File with config to broadcast. Can be used multiple times
# ''')
# )

result = GenericScalar()


Expand Down

0 comments on commit 36ba052

Please sign in to comment.