remove Cylc 7 compatibility mode - #7325
Conversation
| asyncio.run(run(options, workflow_id)) | ||
|
|
||
|
|
||
| async def run( | ||
| parser: COP, options: 'Values', workflow_id: str | ||
| options: 'Values', workflow_id: str |
There was a problem hiding this comment.
Off-topic change: A couple of scripts were passing through the unused parser argument which was creating hassle for tests. These have been removed so all commands have the same basic interface.
* Support for running workflows defined by `suite.rc` files was removed in Cylc 8.7 (cylc#7325) * However, Cylc 8 remains `suite.rc` "aware" in order to allow the GUI, Tui and CLI to list and operate on workflows running under older versions. * A version has been set for the removal of this support in line with cylc#7274.
* remove documentation for Cylc 7 compatibility mode * See cylc/cylc-flow#7325 * user guide: add section on cylc versions and compatibility * dictionary
* Support for running workflows defined by `suite.rc` files was removed in Cylc 8.7 (cylc#7325) * However, Cylc 8 remains `suite.rc` "aware" in order to allow the GUI, Tui and CLI to list and operate on workflows running under older versions. * A version has been set for the removal of this support in line with cylc#7274.
|
Rebased, deconflicted and addressed feedback. |
* Support for running workflows defined by `suite.rc` files was removed in Cylc 8.7 (cylc#7325) * However, Cylc 8 remains `suite.rc` "aware" in order to allow the GUI, Tui and CLI to list and operate on workflows running under older versions. * A version has been set for the removal of this support in line with cylc#7274.
|
Have now added: 9be3f5d
|
* Support for running workflows defined by `suite.rc` files was removed in Cylc 8.7 (cylc#7325) * However, Cylc 8 remains `suite.rc` "aware" in order to allow the GUI, Tui and CLI to list and operate on workflows running under older versions. * A version has been set for the removal of this support in line with cylc#7274.
hjoliver
left a comment
There was a problem hiding this comment.
Reapproving post @MetRonnie 's much more thorough review! (the changes look good)
| continue | ||
| base_point = point | ||
| break | ||
| base_point = min(self.get_tasks_by_point(), default=None) |
There was a problem hiding this comment.
The keys of self.get_tasks_by_point() are the same as self.active_tasks but with some processing of the values that is not needed here.
| base_point = min(self.get_tasks_by_point(), default=None) | |
| base_point = min(self.active_tasks.keys(), default=None) |
There was a problem hiding this comment.
(Off topic), but yes, this is quite inefficient, I'm not sure how it got like this, will change!
Note, this appears to be the only use of get_tasks_by_point!
There was a problem hiding this comment.
Note #7400 removes the call to get_tasks_by_point from the Cylc 8 branch.
| Jinja2 warnings and turned them into Cylc warnings to ensure they cannot be | ||
| missed. | ||
|
|
||
| To give users a chance to take action, this patch restores the old |
There was a problem hiding this comment.
Unless this was meant to be exhaustive,
| To give users a chance to take action, this patch restores the old | |
| To give users a chance to take action, this patch restores some of the old |
There was a problem hiding this comment.
This comment is scoped to renamed interfaces (not removed or modified interfaces):
From this line above:
Jinja2 renamed a bunch of interfaces. Warnings were added in version 3.0.0,
It should restore all of the renamed interfaces.
There was a problem hiding this comment.
I would add a note that this does not cover all the removed deprecations: https://jinja.palletsprojects.com/en/stable/changes/#version-3-1-0
There was a problem hiding this comment.
Will clarify thus:
| To give users a chance to take action, this patch restores the old | |
| To give users a chance to take action, this patch restores the renamed |
This comment does not pertain to the removed deprecations, only the renamed ones.
* Support for running workflows defined by `suite.rc` files was removed in Cylc 8.7 (cylc#7325) * However, Cylc 8 remains `suite.rc` "aware" in order to allow the GUI, Tui and CLI to list and operate on workflows running under older versions. * A version has been set for the removal of this support in line with cylc#7274.
|
@MetRonnie, please the recent force-push which address feedback above:
@hjoliver, you may want to look over the last four commits:
|
| # flow.cylc present | ||
| (tmp_path / WorkflowFiles.SUITE_RC).unlink() | ||
| with pytest.raises(WorkflowConfigError): | ||
| await validate(ValidateOptions(), str(tmp_path)) |
There was a problem hiding this comment.
This should not raise an error, as flow.cylc only should be ok? (Currently the error is because the file is empty so not a valid config)
There was a problem hiding this comment.
The previous two examples raise WorkflowFilesError, this one raises WorkflowConfigError which demonstrates the flow.cylc file is being accepted.
A little confusing admirably, I've added a match pattern and a comment to clarify.
* Closes cylc#6849 * Remove Cylc 7 compatibility mode. * Remove support for validating, (re)installing or (re)starting `suite.rc` workflows. * Keep support for CLI commands against workflows running under older versions of Cylc 8. * Remove legacy functional tests. * Add integration tests (to ensure `suite.rc` workflows cannot be started). * Remove redundant Parsec `silent` option.
* Closes cylc#6850 * Remove the upper pin on Jinja2 allowing the use of newer versions.
* The flags module contains global variables, generally considered an anti-pattern. * We're down to the last flag, add a note to discourage the addition of any new ones!
* Support for running workflows defined by `suite.rc` files was removed in Cylc 8.7 (cylc#7325) * However, Cylc 8 remains `suite.rc` "aware" in order to allow the GUI, Tui and CLI to list and operate on workflows running under older versions. * A version has been set for the removal of this support in line with cylc#7274.
* Back support Jinja2 interfaces deprecated at 3.0 and removed at 3.1. * Jinja2 renamed a bunch of functions and deprecated their old names. * Our users didn't spot this as Python warnings are not visible to them. * As of cylc#7365 (8.6.6) Python warnings originating from Jinja2 will be turned into Cylc warnings. * To provide a migration window, the old function names will be supported in Cylc 8.7.x and removed in 8.8.0.
* Reinstate support for `flow.cylc -> suite.rc` symlinks (which we've supported to date with no warnings).
* Test will ensure `suit.rc` files remain supported post removal of Cylc 7 compatibility mode until purposefully withdrawn in a later version.
|
(ISC004 lint failure to be fixed on 8.6.x) |
|
LGTM |
|
N.B. I had also noticed |
A set of changes to remove Cylc 7 compatibility mode, issues:
Sibling PRs:
Summary:
suite.rcworkflows.suite.rcworkflows cannot be started).suite.rc"aware" for now, it's needed for GUI/Tui/CLI.Check List
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfg(andconda-environment.ymlif present).?.?.xbranch.