`~_W'FLOW_NAME` ⇒ `~_W'FLOW_ID` and `~_W'FLOW_NAME` re-instated as Cylc7 consistent #4455

wxtim · 2021-10-07T08:45:17Z

These changes:

close workflow name environment variable #4438
close Swap REG for FLOW (or similar) in command help #3817
Rename CYLC_WORKFLOW_NAME to CYLC_WORKFLOW_ID (to match universal ID and CLI use).
Add a CYLC_WORKFLOW_NAME variable which is the same but with the run name removed.

Requirements check-list

I have read CONTRIBUTING.md and added my name as a Code Contributor.
Contains logically grouped changes (else tidy your branch by rebase).
Does not contain off-topic changes (use other PRs for other changes).
Applied any dependency changes to both setup.py and
conda-environment.yml.
Appropriate tests are included (unit and/or functional).
Appropriate change log entry included.
(master branch) I have opened a documentation PR at companion to https://github.com/cylc/cylc-flow/pull/4455 cylc-doc#305.

If merging, please squash-merge.

Made CYLC_WORKFLOW_ID = CYLC_WORKFLOW_NAME - trailing "runX"

MetRonnie

I'm seeing empty files Numbat, Quokka and Wambenger

cylc/flow/config.py

cylc/flow/pathutil.py

tests/functional/lib/bash/test_header

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>

wxtim · 2021-10-07T13:37:41Z

I'm seeing empty files Numbat, Quokka and Wambenger

Fail, on my part.

cylc/flow/pathutil.py

hjoliver

@wxtim could you also replace REG with WORKFLOW_ID in CLI help docstrings? That would close #3817 as well (which is linked as part of the proposal in #4438)

MetRonnie · 2021-10-08T09:15:03Z

Seeing as most commands infer the run number now, I think it would be more appropriate to rename REG to WORKFLOW? The description could be "Workflow name or ID".

And for the offline commands, which accept paths as well as the workflow name/id, perhaps we should use WORKFLOW | PATH?

tests/flakyfunctional/xtriggers/00-wall_clock.t

tests/flakyfunctional/xtriggers/00-wall_clock/flow.cylc

cylc/flow/scripts/clean.py

cylc/flow/scripts/config.py

cylc/flow/scripts/install.py

cylc/flow/scripts/reload.py

cylc/flow/scripts/subscribe.py

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>

wxtim · 2021-10-11T06:40:23Z

Thanks for all your help with this @MetRonnie - I seemed to have missed a fair few things. 😢

Having said which a quick play showed me that I'd missed cylc play so I have changed that too.

MetRonnie

Thanks

hjoliver

Looks good, thanks @wxtim

oliver-sanders

Sorry for the post-humorous review.

I think there are lots of CYLC_WORKFlOW_NAME variables left which weren't renamed to CYLC_WORKFLOW_ID. These will be getting a different value which could cause issues. Because the tests generally don't make use of run names these issues may have been missed here.

cylc/flow/option_parsers.py

oliver-sanders · 2021-10-13T12:48:41Z

cylc/flow/pathutil.py

+    >>> get_workflow_name_from_id('my_other_workflow')
+    'my_other_workflow'
+    """
+    return re.sub(rf'{re.escape(os.sep)}run\d+$', '', workflow_id)


This will not work for named runs, need to use the _cylc-install dir to determine the workflow name.

(note if there is no _cylc-install dir then the workflow name == workflow id)

oliver-sanders · 2021-10-13T12:50:02Z

cylc/flow/scheduler_cli.py

@@ -91,7 +93,7 @@
 """


-FLOW_NAME_ARG_DOC = ("REG", "Workflow name")
+FLOW_NAME_ARG_DOC = ("WORKFLOW", "Workflow name or ID")


I think this should just be "Workflow ID".

The workflow name would only work if using numbered runs and the run you want to use is the latest.

Do you not think that will be quite a large portion of use cases?

oliver-sanders · 2021-10-13T12:50:16Z

cylc/flow/scripts/broadcast.py

@@ -224,7 +224,7 @@ def get_option_parser():
    """CLI for "cylc broadcast"."""
    parser = COP(
        __doc__, comms=True,
-        argdoc=[('REG', "Workflow name")]
+        argdoc=[('WORKFLOW', 'Workflow name or ID')]


"Workflow ID"?

oliver-sanders · 2021-10-13T12:50:25Z

cylc/flow/scripts/cat_log.py

@@ -225,7 +225,7 @@ def get_option_parser():
    parser = COP(
        __doc__,
        argdoc=[
-            ("REG", "Workflow name"),
+            ("WORKFLOW", "Workflow name or ID"),


"Workflow ID"?

tests/functional/clock-expire/00-basic/flow.cylc

companion to cylc/cylc-flow#4455

…ylc.reviw * 'master' of github.com:cylc/cylc-doc: Apply suggestions from code review Update src/glossary.rst fix some CYLC_WORKFLOW_NAME ⇒ ID companion to cylc/cylc-flow#4455 Make config upgrade section slightly more concise Add more detail to back compat section Remove obsolete caveat Add line about Cylc 7 back compat mode

wxtim added 5 commits October 6, 2021 15:27

write test for new WORKFLOW_ID=foo/run1 and WORKFLOW_NAME=foo

0208942

Renamed CYLC_WORKFLOW_NAME ⇒ CYLC_WORKFLOW_ID

85d7534

Made CYLC_WORKFLOW_ID = CYLC_WORKFLOW_NAME - trailing "runX"

allow test_header init_workflow to set whether to use --no-run-name

0aefd60

fixed test for CYLC_WORKFLOW_ID

0a3bbbd

CHANGES.md added

56695ca

wxtim changed the title ~~4438.rename cylc s UI te name~~ ~_W'FLOW_NAME ⇒ ~_W'FLOW_ID and ~_W'FLOW_NAME re-instated as Cylc7 consistent Oct 7, 2021

wxtim requested review from MetRonnie and hjoliver October 7, 2021 08:50

wxtim self-assigned this Oct 7, 2021

wxtim added the could be better Not exactly a bug, but not ideal. label Oct 7, 2021

wxtim added this to the cylc-8.0rc1 milestone Oct 7, 2021

wxtim added a commit to wxtim/cylc-doc that referenced this pull request Oct 7, 2021

companion to cylc/cylc-flow#4455

2087f56

wxtim mentioned this pull request Oct 7, 2021

companion to https://github.com/cylc/cylc-flow/pull/4455 cylc/cylc-doc#305

Merged

MetRonnie modified the milestones: cylc-8.0rc1, cylc-8.0b3 Oct 7, 2021

MetRonnie requested changes Oct 7, 2021

View reviewed changes

cylc/flow/config.py Show resolved Hide resolved

cylc/flow/pathutil.py Outdated Show resolved Hide resolved

cylc/flow/pathutil.py Outdated Show resolved Hide resolved

tests/functional/lib/bash/test_header Show resolved Hide resolved

wxtim and others added 3 commits October 7, 2021 14:16

Apply suggestions from code review

e76b043

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>

remove unwanted files; add documentation for change to init_workflow

a48be6f

finished rename of runN_remover

ba37e34

wxtim requested a review from MetRonnie October 7, 2021 13:37

MetRonnie reviewed Oct 7, 2021

View reviewed changes

cylc/flow/pathutil.py Outdated Show resolved Hide resolved

hjoliver requested changes Oct 8, 2021

View reviewed changes

added docstring as per @MetRonnie suggestion

33ae4ae

MetRonnie requested changes Oct 8, 2021

View reviewed changes

tests/flakyfunctional/xtriggers/00-wall_clock.t Outdated Show resolved Hide resolved

tests/flakyfunctional/xtriggers/00-wall_clock/flow.cylc Outdated Show resolved Hide resolved

remove accidental PR pollution

bbded6e

MetRonnie approved these changes Oct 8, 2021

View reviewed changes

replaced refs in script docstrings to REG to WORKFLOW/ID/NAME

2b802cd

MetRonnie reviewed Oct 8, 2021

View reviewed changes

wxtim and others added 2 commits October 11, 2021 07:19

Apply suggestions from code review

c51526f

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>

Fixed some review comments; fixed cylc play docstring!

27bcc5a

wxtim requested a review from hjoliver October 11, 2021 06:39

fix test reliant on REG

b6ff733

MetRonnie approved these changes Oct 11, 2021

View reviewed changes

hjoliver approved these changes Oct 11, 2021

View reviewed changes

hjoliver merged commit 63146d5 into cylc:master Oct 11, 2021

wxtim mentioned this pull request Oct 13, 2021

replace CYLC_WORKFLOW_NAME with CYLC_WORKFLOW_ID in job.sh #4467

Merged

7 tasks

oliver-sanders reviewed Oct 13, 2021

View reviewed changes

wxtim deleted the 4438.rename_CYLC_SUITE_NAME branch October 13, 2021 13:38

wxtim added a commit to wxtim/cylc-doc that referenced this pull request Oct 15, 2021

companion to cylc/cylc-flow#4455

145fbf4

hjoliver added a commit to cylc/cylc-doc that referenced this pull request Oct 19, 2021

Merge pull request #305 from wxtim/companion.to.cylc.flow.4455

4c12537

companion to cylc/cylc-flow#4455

wxtim mentioned this pull request Oct 20, 2021

remove CYLC_WORKFLOW_NAME from many tests #4472

Merged

7 tasks

dpmatthews mentioned this pull request Mar 30, 2022

terminology: straighten out usage of "flow" #4786

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`~_W'FLOW_NAME` ⇒ `~_W'FLOW_ID` and `~_W'FLOW_NAME` re-instated as Cylc7 consistent #4455

`~_W'FLOW_NAME` ⇒ `~_W'FLOW_ID` and `~_W'FLOW_NAME` re-instated as Cylc7 consistent #4455

wxtim commented Oct 7, 2021 •

edited by hjoliver

Loading

MetRonnie left a comment

wxtim commented Oct 7, 2021

hjoliver left a comment

MetRonnie commented Oct 8, 2021 •

edited

Loading

wxtim commented Oct 11, 2021

MetRonnie left a comment

hjoliver left a comment

oliver-sanders left a comment

oliver-sanders Oct 13, 2021

oliver-sanders Oct 13, 2021

wxtim Oct 13, 2021

oliver-sanders Oct 13, 2021

oliver-sanders Oct 13, 2021

~_W'FLOW_NAME ⇒ ~_W'FLOW_ID and ~_W'FLOW_NAME re-instated as Cylc7 consistent #4455

~_W'FLOW_NAME ⇒ ~_W'FLOW_ID and ~_W'FLOW_NAME re-instated as Cylc7 consistent #4455

Conversation

wxtim commented Oct 7, 2021 • edited by hjoliver Loading

MetRonnie left a comment

Choose a reason for hiding this comment

wxtim commented Oct 7, 2021

hjoliver left a comment

Choose a reason for hiding this comment

MetRonnie commented Oct 8, 2021 • edited Loading

wxtim commented Oct 11, 2021

MetRonnie left a comment

Choose a reason for hiding this comment

hjoliver left a comment

Choose a reason for hiding this comment

oliver-sanders left a comment

Choose a reason for hiding this comment

oliver-sanders Oct 13, 2021

Choose a reason for hiding this comment

oliver-sanders Oct 13, 2021

Choose a reason for hiding this comment

wxtim Oct 13, 2021

Choose a reason for hiding this comment

oliver-sanders Oct 13, 2021

Choose a reason for hiding this comment

oliver-sanders Oct 13, 2021

Choose a reason for hiding this comment

`~_W'FLOW_NAME` ⇒ `~_W'FLOW_ID` and `~_W'FLOW_NAME` re-instated as Cylc7 consistent #4455

`~_W'FLOW_NAME` ⇒ `~_W'FLOW_ID` and `~_W'FLOW_NAME` re-instated as Cylc7 consistent #4455

wxtim commented Oct 7, 2021 •

edited by hjoliver

Loading

MetRonnie commented Oct 8, 2021 •

edited

Loading