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

Cylc Config json output mode #6275

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Cylc Config json output mode #6275

wants to merge 2 commits into from
202 −5

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Aug 1, 2024 •
edited
Loading

Add the following command to Cylc Config:

$ cylc config --json

Required for cylc/cylc-sphinx-extensions#81.

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.cfg (and conda-environment.yml if present).
  • Tests are included (or explain why tests are not needed).
  • Changelog entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@wxtim wxtim mentioned this pull request Aug 1, 2024
Draft
8 tasks
@wxtim wxtim self-assigned this Aug 1, 2024
@wxtim wxtim added this to the 8.4.0 milestone Aug 1, 2024
@wxtim wxtim added small speculative blue-skies ideas labels Aug 1, 2024
oliver-sanders
oliver-sanders reviewed Aug 2, 2024
View reviewed changes
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

👍

cylc/flow/parsec/config.py Outdated Show resolved Hide resolved
wxtim
wxtim commented Aug 5, 2024
View reviewed changes
cylc/flow/parsec/config.py Outdated Show resolved Hide resolved
@wxtim wxtim marked this pull request as draft August 5, 2024 12:27
@wxtim wxtim marked this pull request as ready for review August 16, 2024 14:21
@MetRonnie

This comment was marked as resolved.

Sign in to view
@wxtim wxtim force-pushed the feat.parsec_to_json_dumper branch from 2d8e721 to 5480b12 Compare August 23, 2024 13:29
@MetRonnie MetRonnie removed the speculative blue-skies ideas label Aug 23, 2024
MetRonnie
MetRonnie previously requested changes Aug 23, 2024
View reviewed changes
cylc/flow/parsec/config.py Outdated Show resolved Hide resolved
cylc/flow/scripts/config.py Outdated Show resolved Hide resolved
@wxtim wxtim requested a review from MetRonnie August 27, 2024 09:25
@wxtim wxtim marked this pull request as draft August 28, 2024 08:17
@wxtim
Copy link
Member Author

wxtim commented Aug 28, 2024

Drafted pending other work this not being a priority compared to bugfixes.

@wxtim
Allow Cylc Config to output a metadata JSON.
02616bc 
Response to review
- Ensure --json help message is useful.
- Make the null value reporting only happen to actual null values.

Prevent --json being combined with print.. options

Tidy up
@wxtim wxtim force-pushed the feat.parsec_to_json_dumper branch from f87fcf0 to 02616bc Compare August 29, 2024 13:03
@wxtim wxtim requested a review from MetRonnie August 29, 2024 13:03
@wxtim wxtim marked this pull request as ready for review August 29, 2024 13:03
@MetRonnie
Copy link
Member

Workflow with implicit task foo

$ cylc config myworkflow -i '[runtime][foo]' --null-value=meow --json
Traceback (most recent call last):
  File "~/.conda/envs/cylc8/bin/cylc", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "~/github/cylc-flow/cylc/flow/scripts/cylc.py", line 703, in main
    execute_cmd(command, *cmd_args)
  File "~/github/cylc-flow/cylc/flow/scripts/cylc.py", line 334, in execute_cmd
    entry_point.load()(*args)
  File "~/github/cylc-flow/cylc/flow/terminal.py", line 298, in wrapper
    wrapped_function(*wrapped_args, **wrapped_kwargs)
  File "~/github/cylc-flow/cylc/flow/scripts/config.py", line 189, in main
    asyncio.run(_main(parser, options, *ids))
  File "~/.conda/envs/cylc8/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "~/.conda/envs/cylc8/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/.conda/envs/cylc8/lib/python3.11/asyncio/base_events.py", line 654, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "~/github/cylc-flow/cylc/flow/scripts/config.py", line 245, in _main
    config.pcfg.idump(
  File "~/github/cylc-flow/cylc/flow/parsec/config.py", line 186, in idump
    self.jdump(mkeys, sparse, oneline, none_str, handle=handle)
  File "~/github/cylc-flow/cylc/flow/parsec/config.py", line 215, in jdump
    cfg.repl_val(cfg, None, none_str)
    ^^^^^^^^^^^^
AttributeError: 'dict' object has no attribute 'repl_val'

@wxtim wxtim dismissed MetRonnie’s stale review August 29, 2024 15:13

Made necessary change

@oliver-sanders
Copy link
Member

oliver-sanders commented Oct 7, 2024 •
edited
Loading

This PR could probably be merged now, however, IMO it's best to keep pairs of PRs open (i.e. unmerged) until the upstream PR has been merged as review comments there could force changes here. Will review together with the upstream PR when both are ready.

The upstream PR is currently a draft, so I'll mark this as draft for now to match.

@oliver-sanders oliver-sanders marked this pull request as draft October 7, 2024 12:57
@wxtim
Copy link
Member Author

wxtim commented Oct 9, 2024

I will get back to this one day, but it's a feature, not a bug...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MetRonnie MetRonnie MetRonnie approved these changes

@oliver-sanders oliver-sanders Awaiting requested review from oliver-sanders

Assignees

@wxtim wxtim

Labels
small
Projects
None yet
Milestone
8.4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@wxtim @MetRonnie @oliver-sanders