Refactor workflow_state xtrig pre-8.3.0-back-compat
#51
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MetRonnie
force-pushed
the
workflow-state
branch
2 times, most recently
from
a1f7b6c
to
f97f314
Compare
hjoliver
force-pushed
the
fix-workflow-state-check
branch
2 times, most recently
from
77c44bd
to
960fb61
Compare
MetRonnie
commented
Jun 5, 2024
Comment on lines
-282
to
+298
| outputs_map = json.loads(row[2]) | ||
| if is_message: | ||
| # task message | ||
| try: | ||
| outputs = list(outputs_map.values()) | ||
| except AttributeError: | ||
| # Cylc 8 pre 8.3.0 back-compat: list of output messages | ||
| outputs = list(outputs_map) | ||
| outputs: Union[Dict[str, str], List[str]] = json.loads(row[2]) | ||
| if isinstance(outputs, dict): | ||
| messages: Iterable[str] = outputs.values() | ||
| else: | ||
| # task output | ||
| outputs = list(outputs_map) | ||
| # Cylc 8 pre 8.3.0 back-compat: list of output messages | ||
| messages = outputs | ||
| if warn_output_fallback: | ||
| LOG.warning(output_fallback_msg) | ||
| warn_output_fallback = False |
There was a problem hiding this comment.
Clearer distinction between messages and outputs, added warning for 8.0.0-8.3.0 tasks
| @@ -52,10 +52,7 @@ cmp_ok "${TEST_NAME}.stderr" - </dev/null | |||
| #------------------------------------------------------------------------------- | |||
| TEST_NAME="${TEST_NAME_BASE}-section2" | |||
| run_ok "${TEST_NAME}" cylc config -d --item=[runtime] "${WORKFLOW_NAME}" | |||
| # Crude sorting to handle against change of dict order when new items added: | |||
There was a problem hiding this comment.
This output is deterministic and should not be sorted, otherwise we're not checking that settings are in the right sections
There was a problem hiding this comment.
git blame says the sorting dates back to 2015. Maybe we weren't using an ordered dict then...
MetRonnie
commented
Jun 5, 2024
Comment on lines
+395
to
+400
| LOG.warning( | ||
| "(8.3.0) Deprecated function signature used for " | ||
| "workflow_state xtrigger was automatically upgraded. Please " | ||
| "alter your workflow to use the new syntax:\n" | ||
| f" {old_sig_str} --> {upg_sig_str}" | ||
| ) |
There was a problem hiding this comment.
Added deprecation warning, also added grep test in tests/functional/workflow-state/11-multi.t
MetRonnie
commented
Jun 5, 2024
Comment on lines
-447
to
462
| task_outputs = json.loads(task_outputs) | ||
| # BACK COMPAT: In Cylc >8.0.0,<8.3.0, only the task | ||
| # messages were stored in the DB as a list. | ||
| # from: 8.0.0 | ||
| # to: 8.3.0 | ||
| outputs: Union[ | ||
| Dict[str, str], List[str] | ||
| ] = json.loads(task_outputs) | ||
| messages = ( | ||
| outputs.values() if isinstance(outputs, dict) | ||
| else outputs | ||
| ) | ||
| return ( | ||
| 'satisfied from database' | ||
| if output in task_outputs | ||
| if output_msg in messages | ||
| else False | ||
| ) |
hjoliver
reviewed
Jun 5, 2024
| if flow_nums.intersection(fnums): | ||
| for msg in json.loads(outputs_str): | ||
| itask.state.outputs.set_message_complete(msg) | ||
| self._load_historical_outputs(itask) |
hjoliver
reviewed
Jun 5, 2024
Comment on lines
+75
to
+76
| return _workflow_state_backcompat( | ||
| suite, task, point, offset, status, message, cylc_run_dir |
There was a problem hiding this comment.
Good (I initially did this prior to adding the backcompat function).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Against cylc#5809