stop after cycle point: support offsets #5943
Merged
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
oliver-sanders
commented
Jan 29, 2024
| @@ -699,6 +700,30 @@ class CylcConfigValidator(ParsecValidator): | |||
| '-0830': 'UTC minus 8 hours and 30 minutes.' | |||
| } | |||
| ), | |||
| V_CYCLE_POINT_WITH_OFFSETS: ( | |||
There was a problem hiding this comment.
This is a new parsec validator type which does the same thing as V_STRING.
It exists purely for documentation purposes.
oliver-sanders
commented
Jan 29, 2024
| @@ -993,6 +993,7 @@ def command_stop( | |||
| point = TaskID.get_standardised_point(cycle_point) | |||
| if point is not None and self.pool.set_stop_point(point): | |||
| self.options.stopcp = str(point) | |||
| self.config.stop_point = point | |||
There was a problem hiding this comment.
Spotted during testing, the config stop_point isn't updated when it is modified via the CLI, but the config hold_point is.
oliver-sanders
commented
Jan 29, 2024
oliver-sanders
commented
Jan 29, 2024
wxtim
requested changes
Jan 30, 2024
* Closes cylc#5939 * Support offsets (measured from the ICP) for the `stop after cycle point` to mirror the behaviour of the `final cycle point`. * Add integration test to lock-down `stop after cycle point` interactions.
MetRonnie
reviewed
Jan 31, 2024
Comment on lines
-699
to
+713
| id="From config by default" | ||
| id="stopcp" | ||
| ), | ||
| pytest.param( | ||
| '1993', '1066', '1066', '1066', None, | ||
| id="From options" | ||
| id="stop-cp-and-cli-option" | ||
| ), | ||
| pytest.param( | ||
| '1993', 'reload', '1993', None, None, | ||
| id="From cfg if --stopcp=reload on restart" | ||
| id="stop-cp-and-cli-reload-option" |
There was a problem hiding this comment.
I think the previous ids were more descriptive
There was a problem hiding this comment.
fair enough, I changed them to something more CLI friendly, suggestions welcome.
There was a problem hiding this comment.
Having a short id plus a more descriptive comment would be reasonable IMO
Co-authored-by: Ronnie Dutta <[email protected]>
MetRonnie
reviewed
Feb 1, 2024
Co-authored-by: Ronnie Dutta <[email protected]>
MetRonnie
reviewed
Feb 2, 2024
Co-authored-by: Ronnie Dutta <[email protected]>
MetRonnie
approved these changes
Feb 5, 2024
wxtim
approved these changes
Feb 9, 2024
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.
stop after cycle pointto mirror the behaviour of thefinal cycle point.stop after cycle pointinteractions.Check List
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfg(andconda-environment.ymlif present).CHANGES.mdentry included if this is a change that can affect users?.?.xbranch.