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

Fix.no validation of outputs from graph singletons #6583

Open
wants to merge 10 commits into
base: 8.4.x
Choose a base branch
from
Open

Fix.no validation of outputs from graph singletons #6583

wants to merge 10 commits into from
+108 −31

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Jan 30, 2025 •
edited
Loading

Closes #6508

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 marked this pull request as draft January 30, 2025 09:38
@wxtim wxtim self-assigned this Jan 30, 2025
@wxtim wxtim added the bug Something is wrong :( label Jan 30, 2025
@wxtim wxtim added this to the 8.4.1 milestone Jan 30, 2025
@wxtim wxtim linked an issue Jan 30, 2025 that may be closed by this pull request
lone custom outputs ignored #6508
Open
@wxtim wxtim force-pushed the fix.no_validation_of_outputs_from_graph_singletons branch 5 times, most recently from 9ab665a to e9b7e6e Compare February 3, 2025 11:30
@wxtim wxtim marked this pull request as ready for review February 3, 2025 12:38
@hjoliver
Copy link
Member

hjoliver commented Feb 3, 2025

(See new comment on the issue #6508 (comment) )

hjoliver
hjoliver requested changes Feb 3, 2025
View reviewed changes
@wxtim wxtim requested a review from hjoliver February 5, 2025 13:11
@wxtim wxtim force-pushed the fix.no_validation_of_outputs_from_graph_singletons branch from 793bb30 to bdadd4b Compare February 5, 2025 15:41
@wxtim wxtim marked this pull request as draft February 6, 2025 10:05
@wxtim wxtim force-pushed the fix.no_validation_of_outputs_from_graph_singletons branch from ad1fb9f to c162aaf Compare February 6, 2025 10:22
@wxtim wxtim marked this pull request as ready for review February 7, 2025 07:16
hjoliver
hjoliver previously requested changes Feb 10, 2025
View reviewed changes
hjoliver
hjoliver reviewed Feb 10, 2025
View reviewed changes
wxtim
wxtim commented Feb 11, 2025
View reviewed changes
@wxtim wxtim requested a review from hjoliver February 11, 2025 11:24
@wxtim wxtim force-pushed the fix.no_validation_of_outputs_from_graph_singletons branch from 03de62d to 8066dbf Compare February 17, 2025 11:11
wxtim
wxtim commented Feb 18, 2025
View reviewed changes
@wxtim wxtim force-pushed the fix.no_validation_of_outputs_from_graph_singletons branch from e9d8ffc to 8440029 Compare February 18, 2025 10:18
oliver-sanders
oliver-sanders reviewed Feb 19, 2025
View reviewed changes
@wxtim wxtim force-pushed the fix.no_validation_of_outputs_from_graph_singletons branch from 19f1bb0 to 8699e70 Compare February 19, 2025 10:53
@wxtim wxtim force-pushed the fix.no_validation_of_outputs_from_graph_singletons branch from 0175e35 to b1c9606 Compare February 19, 2025 14:35
wxtim and others added 7 commits February 19, 2025 15:08
@wxtim
Add test for situation where undefined optional outputs fail to raise…
ee842bf 
… an error everywhere in graph.
@wxtim
Ensure that undefined optional outputs raise an error everywhere in g…
035afd7 
…raph.
@wxtim
Response to reviews
1e59531 
Update changes.d/6583.fix.md

Co-authored-by: Hilary James Oliver <[email protected]>

Update cylc/flow/config.py

Co-authored-by: Hilary James Oliver <[email protected]>

respond to Hilary's review: Don't check all outputs only terminals.

use regexs to sort out issues with qualifiers, suicide triggers &c

mypy
@wxtim
remove stray walrus
08f2836
@wxtim
Response to review
f125c0f
@wxtim
Fix a terminal graph items bug
c496207
@wxtim
fix some more test cases for terminal finder
152ddb5
@wxtim wxtim force-pushed the fix.no_validation_of_outputs_from_graph_singletons branch from b1c9606 to 152ddb5 Compare February 19, 2025 15:38
@oliver-sanders oliver-sanders modified the milestones: 8.4.1, 8.4.x Feb 19, 2025
@wxtim wxtim marked this pull request as draft February 21, 2025 11:13
@wxtim wxtim marked this pull request as ready for review February 24, 2025 11:54
@wxtim wxtim requested a review from oliver-sanders February 26, 2025 16:37
@oliver-sanders oliver-sanders dismissed hjoliver’s stale review March 11, 2025 13:29

Review comments addressed.

oliver-sanders
oliver-sanders approved these changes Mar 11, 2025
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.

👍

Plz squash merge.

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 approved these changes

@hjoliver hjoliver Awaiting requested review from hjoliver

Assignees

@wxtim wxtim

Labels
bug Something is wrong :(
Projects
None yet
Milestone
8.4.x
Development

Successfully merging this pull request may close these issues.

lone custom outputs ignored
3 participants
@wxtim @hjoliver @oliver-sanders