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

Add BroadcastCyclePoint type to schema #5007

Merged
merged 4 commits into from
Aug 3, 2022
Merged

Conversation

MetRonnie
Copy link
Member

@MetRonnie MetRonnie commented Jul 22, 2022

This is a small change with no associated Issue.

Needed for proper mutation form validation - cylc/cylc-ui#1073

The Broadcast cycle point option can be a cycle point or * (supports either of those two but not cycle point globs)

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.cfg and conda-environment.yml.
  • Does not need tests (why?).
  • Change log entry included
  • No documentation update required.
  • [if bugfix] PRs raised to both master and the relevant bugfix branch. Merge 8.0.x into master #5039

@MetRonnie MetRonnie added this to the cylc-8.0.1 milestone Jul 22, 2022
@MetRonnie MetRonnie self-assigned this Jul 22, 2022
@MetRonnie MetRonnie mentioned this pull request Jul 22, 2022
6 tasks
@MetRonnie
Copy link
Member Author

MetRonnie commented Jul 22, 2022

The Broadcast cycle point option can be a cycle point or * (supports either of those two but not cycle point globs)

Or can it? This was the impression I got from the existing code but it isn't explicitly stated

@MetRonnie MetRonnie added the question Flag this as a question for the next Cylc project meeting. label Jul 22, 2022
@MetRonnie
Copy link
Member Author

(Test failure due to failed coverage fetch only)

@oliver-sanders
Copy link
Member

Or can it? This was the impression I got from the existing code but it isn't explicitly stated

I think * is just a marker for "all cycles" not a glob.

try:
point_string = standardise_point_string(point_string)
except PointParsingError:
if point_string != '*':
bad_point_strings.append(point_string)
bad_point = True

@oliver-sanders oliver-sanders removed the question Flag this as a question for the next Cylc project meeting. label Jul 29, 2022
@MetRonnie MetRonnie requested a review from datamel August 1, 2022 16:27
@MetRonnie
Copy link
Member Author

@datamel Could you try this out? Try using the broadcast mutation edit form in the UI, you should find that the broadcast cycle point option can be a cycle point or * (supports either of those two but not cycle point globs)

@datamel
Copy link
Contributor

datamel commented Aug 2, 2022

@MetRonnie, have you this working in the gui? I get an Invalid Cycle Point for *.

@MetRonnie
Copy link
Member Author

MetRonnie commented Aug 2, 2022

Working for me, only errors when try to use * and another character, or some other invalid letter etc.

demo7

@datamel
Copy link
Contributor

datamel commented Aug 2, 2022

Thanks, maybe I have something up with my build. I get an invalid cycle point and then if I add a space I get a Cannot contain spaces error. I will re-build and see if I can spot my mistake.

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.

A fresh environment has solved my problems with this. Thanks @MetRonnie.

@MetRonnie
Copy link
Member Author

@datamel If you're happy with my conflict-resolve merge commit then hit the merge button

@datamel datamel merged commit 7d2189e into cylc:8.0.x Aug 3, 2022
@MetRonnie MetRonnie deleted the schema branch August 3, 2022 10:06
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.

3 participants