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

Don't record satisfied wall clock triggers in the DB #5923

Merged
merged 7 commits into from
Feb 5, 2024
Merged

Don't record satisfied wall clock triggers in the DB #5923

merged 7 commits into from
Feb 5, 2024

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Jan 15, 2024 •
edited
Loading

Closes #5911

n.b. I tried to find a more sophisticated method than startswith('wall_clock(...') to identify the correct xtriggers, but I think this will do.

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).
  • This is a change to internals, so it doesn't need a docs/changelog update.
  • Raised against 8.2.x

@oliver-sanders
Copy link
Member

Execution/submission retries are also implemented via wall_clock xtriggers.

Worth checking/testing that these aren't written to the DB.

@MetRonnie MetRonnie changed the title Don't record satisfied xtriggers in the DB Don't record satisfied clock triggers in the DB Jan 15, 2024
@MetRonnie MetRonnie changed the title Don't record satisfied clock triggers in the DB Don't record satisfied wall clock triggers in the DB Jan 15, 2024
@wxtim wxtim requested a review from MetRonnie January 15, 2024 15:53
@wxtim wxtim self-assigned this Jan 16, 2024
@wxtim wxtim added this to the cylc-8.2.5 milestone Jan 16, 2024
@wxtim wxtim linked an issue Jan 16, 2024 that may be closed by this pull request
Don't record satisfied clock triggers in the DB #5911
Closed
@wxtim wxtim marked this pull request as draft January 16, 2024 15:25
@wxtim wxtim force-pushed the feat.dont_record_satisfied_clock_triggers branch from cd21f60 to e51cfe9 Compare January 26, 2024 10:59
@wxtim wxtim marked this pull request as ready for review January 26, 2024 11:20
@wxtim wxtim requested a review from oliver-sanders January 26, 2024 11:20
@oliver-sanders oliver-sanders requested review from hjoliver and oliver-sanders and removed request for oliver-sanders and hjoliver January 29, 2024 12:47
MetRonnie

This comment was marked as resolved.

Sign in to view

MetRonnie
MetRonnie approved these changes Jan 31, 2024
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.

Not merging pending async timeout comment above

@wxtim
Copy link
Member Author

wxtim commented Feb 1, 2024

@oliver-sanders - quick check that 94f3a76 addresses your comments?

@oliver-sanders oliver-sanders merged commit 57fe4ae into cylc:8.2.x Feb 5, 2024
25 checks passed
@oliver-sanders
Copy link
Member

@wxtim, make sure we don't end up with two copies of the complete fixture in the sync PR.

@wxtim wxtim deleted the feat.dont_record_satisfied_clock_triggers branch February 6, 2024 15:01
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

@MetRonnie MetRonnie MetRonnie approved these changes

Assignees

@wxtim wxtim

Labels
None yet
Projects
None yet
Milestone
cylc-8.2.5
Development

Successfully merging this pull request may close these issues.

Don't record satisfied clock triggers in the DB
3 participants
@wxtim @oliver-sanders @MetRonnie