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

Ensure jobs are created if task is in waiting state #6176

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Jun 27, 2024

Closes #6172

Repro:

[scheduling]
    [[graph]]
        R1 = foo
[runtime]
    [[foo]]
        platform = borked
        submission retry delays = 5*PT10S

With global.cylc:

[platforms]
    [[borked]]
        hosts = bogus

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).
  • CHANGES.md 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 added the bug Something is wrong :( label Jun 27, 2024
@wxtim wxtim added this to the 8.3.1 milestone Jun 27, 2024
@wxtim wxtim self-assigned this Jun 27, 2024
@wxtim wxtim force-pushed the fix.submit-failed_tasks_not_showing branch from b3b003c to c6a52e6 Compare June 27, 2024 16:15
@@ -1536,10 +1536,11 @@ def _insert_task_job(
job_conf = itask.jobs[-1]

# insert job into data store
job_status = 'submitted' if submit_status == 0 else 'submit-failed'
self.data_store_mgr.insert_job(
Copy link
Member Author

Choose a reason for hiding this comment

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

This method takes a job status, not a task status. If the task status is 'waiting' it returns None and doesn't register the job with the data store.

@wxtim

This comment was marked as off-topic.

@oliver-sanders

This comment was marked as off-topic.

@wxtim wxtim force-pushed the fix.submit-failed_tasks_not_showing branch from a4b96c6 to 0667309 Compare June 28, 2024 07:57
@wxtim wxtim force-pushed the fix.submit-failed_tasks_not_showing branch 2 times, most recently from 529429d to 5b406d8 Compare June 28, 2024 08:04
@wxtim wxtim marked this pull request as draft July 1, 2024 10:08
@wxtim wxtim marked this pull request as ready for review July 1, 2024 11:41
@wxtim wxtim requested a review from oliver-sanders July 1, 2024 11:41
@wxtim wxtim changed the title do not pass task status to DataStoreMgr.insert_job Ensure jobs are created if task is in waiting state Jul 1, 2024
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.

Makes sense, tested as working 👍

tests/integration/test_task_events_mgr.py Outdated Show resolved Hide resolved
tests/integration/test_task_events_mgr.py Outdated Show resolved Hide resolved
tests/integration/test_task_events_mgr.py Outdated Show resolved Hide resolved
@oliver-sanders
Copy link
Member

(rebase to pick up flake8 fixes on upstream/8.3.x)

@wxtim wxtim force-pushed the fix.submit-failed_tasks_not_showing branch from 7aef19a to 98620f1 Compare July 2, 2024 10:37
@wxtim wxtim requested a review from oliver-sanders July 2, 2024 10:37
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.

LGTM

tests/integration/test_task_events_mgr.py Outdated Show resolved Hide resolved
tests/integration/test_task_events_mgr.py Show resolved Hide resolved
tests/integration/test_task_events_mgr.py Outdated Show resolved Hide resolved
tests/integration/test_task_events_mgr.py Outdated Show resolved Hide resolved
@wxtim wxtim requested a review from oliver-sanders July 2, 2024 11:01
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.

Tested manually

cylc/flow/task_events_mgr.py Outdated Show resolved Hide resolved
tests/integration/test_task_events_mgr.py Show resolved Hide resolved
@wxtim wxtim requested a review from MetRonnie July 3, 2024 12:23
@MetRonnie MetRonnie linked an issue Jul 3, 2024 that may be closed by this pull request
@wxtim wxtim force-pushed the fix.submit-failed_tasks_not_showing branch from c584830 to a8d39eb Compare July 4, 2024 11:32
@wxtim wxtim requested a review from oliver-sanders July 4, 2024 11:33
Update tui test KGO

added a test

Apply suggestions from code review

Co-authored-by: Oliver Sanders <[email protected]>
@wxtim wxtim force-pushed the fix.submit-failed_tasks_not_showing branch from a8d39eb to c0ae6b8 Compare July 4, 2024 11:34
@MetRonnie MetRonnie merged commit d0bc4cb into cylc:8.3.x Jul 4, 2024
20 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
Development

Successfully merging this pull request may close these issues.

submit-failed tasks not showing
3 participants