Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cylc-rose run twice bug #4502

Merged
merged 6 commits into from
Nov 19, 2021
Merged

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Nov 10, 2021

This is a small change with no associated Issue:

Cylc was forced to run cylc-rose twice under some circumstances to give uninstalled workflows access to CLI variables. This PR passes CLI variable to Parsec instead.

Partner of cylc/cylc-rose#92 - Both are required for testing.
It replaces #4462 .

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.
  • Already covered by existing tests, these are in the Cylc Rose repo.
  • No Change log entry included because this PR makes existing functionality work slighty faster and without a confusing double error message.
  • No documentation update required.

@wxtim wxtim self-assigned this Nov 10, 2021
@wxtim wxtim modified the milestones: cylc-8.0b3, cylc-8.0rc1 Nov 10, 2021
@wxtim wxtim changed the title fixed parsec run twice bug cylc-rose run twice bug Nov 12, 2021
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partial review

cylc/flow/parsec/config.py Show resolved Hide resolved
cylc/flow/parsec/fileparse.py Outdated Show resolved Hide resolved
@wxtim wxtim requested a review from MetRonnie November 15, 2021 10:58
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little confused about how this pairs with cylc/cylc-rose#92 - there doesn't seem to be any changes to the entry points there. Had the change to the entry point(s) already happened in an earlier PR?

cylc/flow/templatevars.py Outdated Show resolved Hide resolved
@wxtim
Copy link
Member Author

wxtim commented Nov 16, 2021

I am a little confused about how this pairs with cylc/cylc-rose#92 - there doesn't seem to be any changes to the entry points there. Had the change to the entry point(s) already happened in an earlier PR?

The functionality this PR uses has existed in Cylc Rose for a long time, but hadn't been used by the code in parsec/fileparse before - it should have been, hence the PR

@wxtim wxtim marked this pull request as draft November 16, 2021 16:27
@wxtim wxtim marked this pull request as ready for review November 16, 2021 16:37
@wxtim wxtim requested a review from MetRonnie November 16, 2021 16:37
@MetRonnie

This comment has been minimized.

@wxtim wxtim force-pushed the parsec.run.twice.bug branch from 4ed30d5 to 5ee405a Compare November 19, 2021 13:45
@oliver-sanders oliver-sanders merged commit dbaf7ef into cylc:master Nov 19, 2021
@wxtim wxtim deleted the parsec.run.twice.bug branch March 22, 2022 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants