Skip to content

Add --format=json option to cylc broadcast command#6892

Merged
wxtim merged 1 commit intocylc:8.5.xfrom
MetRonnie:broadcast-json
Aug 11, 2025
Merged

Add --format=json option to cylc broadcast command#6892
wxtim merged 1 commit intocylc:8.5.xfrom
MetRonnie:broadcast-json

Conversation

@MetRonnie
Copy link
Copy Markdown
Member

@MetRonnie MetRonnie commented Jul 30, 2025

Closes #6873

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).
  • No dependency changes
  • Tests are included
  • Changelog entry included if this is a change that can affect users
  • No docs needed
  • Very low risk feature hence on 8.5.x branch

@MetRonnie MetRonnie added this to the 8.5.1 milestone Jul 30, 2025
@MetRonnie MetRonnie self-assigned this Jul 30, 2025
@MetRonnie MetRonnie linked an issue Jul 30, 2025 that may be closed by this pull request
Copy link
Copy Markdown
Member

@wxtim wxtim left a comment

Choose a reason for hiding this comment

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

It all works, and it's an elegant test. I wonder if it might be faster set up using the integration test infrastructure then just running the cli under test with a subprocess.run. Don't think it's worth spending time on.

Comment on lines +321 to +345
parser.add_option(
'--json',
help=(
"With -d/--display or -k/--display-task, write out "
"the broadcast config structure in JSON format."
),
action='store_true',
default=False,
dest='json',
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In an attempt to standardise the Cylc CLI options, can we have --format=json, -t=json.

Context: #2972

Deprecate `--raw` in favor of `--format=raw`
@MetRonnie MetRonnie changed the title Add --json option to cylc broadcast command Add --format=json option to cylc broadcast command Aug 4, 2025
Copy link
Copy Markdown
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.

LGTM

action="store_true", default=False, dest="raw")

parser.add_option(
'--format',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider adding the short option -t.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I had a look at scripts that we might implement --format for:

  • broadcast: No conflict for -t in this case. However...
  • dump: -t is --tasks
  • list: -t is --tree
  • play: (which already has --format) -t is --start-task
  • graph: -t is --transpose
  • workflow-state: -t is --task

So I'm not sure there is going to be consistency with scan

@wxtim wxtim merged commit b604a79 into cylc:8.5.x Aug 11, 2025
28 checks passed
@MetRonnie MetRonnie deleted the broadcast-json branch August 12, 2025 08:42
@MetRonnie MetRonnie modified the milestones: 8.5.1, 8.5.2 Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cylc broadcast --display JSON output

3 participants