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.

Sign up for GitHub

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

Jump to bottom

<datetime>-<start/restart/reload>.cylc contain processed, sparse config. #4521

Merged
merged 10 commits into from
Dec 6, 2021
Merged

<datetime>-<start/restart/reload>.cylc contain processed, sparse config. #4521

merged 10 commits into from
Dec 6, 2021

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Nov 17, 2021 •
edited
Loading

Closes #4407

Made the <datetime>-<start/restart/reload>.cylc use pcfg.idump() so that these files contain the processed and sparse config.

To do this I allowed config.pcfg.idump to take a handle kwarg to pass to printcfg.

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 - Worryingly this change didn't actually seem to break any existing tests.
  • Appropriate change log entry included.
  • (master branch) I have opened a documentation PR at companion to https://github.com/cylc/cylc-flow/pull/4521 cylc-doc#320.

@wxtim
Allow processed config idump to take a handle to pass to printcfg
61e59ed 
Made the ``<datetime>-<start/restart/reload>.cylc use pcfg.idump()
  to have the processed and sparse config dumped.
@wxtim wxtim self-assigned this Nov 17, 2021
@wxtim wxtim added the small label Nov 17, 2021
@wxtim wxtim modified the milestones: cylc-8.0rc1, cylc-8.0.0 Nov 17, 2021
@wxtim wxtim linked an issue Nov 17, 2021 that may be closed by this pull request
Review contents of log/flow-config directory #4407
Closed
3 tasks
@wxtim wxtim changed the title Allow processed config idump to take a handle to pass to printcfg <datetime>-<start/restart/reload>.cylc contain sparse config. Nov 17, 2021
@wxtim wxtim changed the title <datetime>-<start/restart/reload>.cylc contain sparse config. <datetime>-<start/restart/reload>.cylc contain processed, sparse config. Nov 17, 2021
@wxtim wxtim marked this pull request as draft November 17, 2021 10:10
@wxtim wxtim marked this pull request as ready for review November 17, 2021 13:09
@wxtim wxtim requested review from hjoliver and datamel November 17, 2021 13:09
wxtim added a commit to wxtim/cylc-doc that referenced this pull request Nov 17, 2021
@wxtim
companion to cylc/cylc-flow#4521
98e823e
@wxtim wxtim mentioned this pull request Nov 17, 2021
Merged
@wxtim wxtim requested a review from oliver-sanders November 19, 2021 11:03
@oliver-sanders oliver-sanders removed their request for review November 19, 2021 16:13
hjoliver added a commit to cylc/cylc-doc that referenced this pull request Nov 22, 2021
@hjoliver
Merge pull request #320 from wxtim/parsed_config_files_should_contain…
8c58043 
…_the_output_of_cylc_config

companion to cylc/cylc-flow#4521
@wxtim wxtim requested review from oliver-sanders and removed request for hjoliver November 22, 2021 09:33
@wxtim wxtim requested review from MetRonnie and oliver-sanders and removed request for oliver-sanders December 1, 2021 09:03
MetRonnie
MetRonnie reviewed Dec 1, 2021
View reviewed changes
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.

Tested: working as advertised

CHANGES.md Outdated Show resolved Hide resolved
cylc/flow/parsec/util.py Outdated Show resolved Hide resolved
@wxtim wxtim requested a review from MetRonnie December 2, 2021 09:57
MetRonnie
MetRonnie approved these changes Dec 2, 2021
View reviewed changes
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 have deconflicted the changelog (and reworded the entry a bit)

datamel
datamel approved these changes Dec 6, 2021
View reviewed changes
Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

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

Thanks @wxtim. Working as described for me. Only minor typo found.

cylc/flow/parsec/util.py Outdated Show resolved Hide resolved
MetRonnie
MetRonnie reviewed Dec 6, 2021
View reviewed changes
CHANGES.md Outdated
Comment on lines 66 to 68
[#4526](https://github.com/cylc/cylc-flow/pull/4526) - Prevent runN and run\d+
being allowed as installation target names.

Copy link
Member

Choose a reason for hiding this comment

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

Duplicate entry

wxtim
wxtim commented Dec 6, 2021
View reviewed changes
CHANGES.md Outdated Show resolved Hide resolved
@wxtim wxtim merged commit 050db73 into cylc:master Dec 6, 2021
@wxtim wxtim deleted the parsed_config_files_should_contain_the_output_of_cylc_config branch December 6, 2021 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliver-sanders oliver-sanders oliver-sanders left review comments

@MetRonnie MetRonnie MetRonnie approved these changes

@datamel datamel datamel approved these changes

Assignees

@wxtim wxtim

Labels
small
Projects
None yet
Milestone
cylc-8.0rc1
Development

Successfully merging this pull request may close these issues.

Review contents of log/flow-config directory
4 participants
@wxtim @oliver-sanders @datamel @MetRonnie