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 an error handling bug in Cylc Scan #5196

Merged
merged 2 commits into from
Oct 21, 2022
Merged

fix an error handling bug in Cylc Scan #5196

merged 2 commits into from
Oct 21, 2022

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Oct 14, 2022 •
edited
Loading

This fixes a bug found by @dpmatthews and not recorded elsewhere.

How to reproduce

  • Create a simple workflow with a long running task on a remote host.
  • Start workflow, noting the PID.
  • Copy the workflow contact file
  • Kill the workflow using the PID
  • Restore the workflow contact file
  • Run Cylc Scan

What you see

Traceback from an uncaught Exception.

    Traceback (most recent call last):
      File "/home/h03/fcm/cylc-8.0.2-1/lib/python3.9/site-
    packages/cylc/flow/network/scan.py", line 464, in graphql_query
        ret = await client.async_request(
      File "/home/h03/fcm/cylc-8.0.2-1/lib/python3.9/site-
    packages/cylc/flow/network/client.py", line 198, in async_request
        self.timeout_handler()
      File "/home/h03/fcm/cylc-8.0.2-1/lib/python3.9/site-
    packages/cylc/flow/network/client.py", line 331, in _timeout_handler
        raise WorkflowStopped(workflow)
    cylc.flow.exceptions.WorkflowStopped: test/migration/run6 is not running

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.
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • If this is a bug fix, PRs raised to both master and the relevant maintenance branch.

@dpmatthews
Copy link
Contributor

Modify ~/.ssh/config to break communications with remote host.

Surely this bit's not relevant?

@wxtim
Copy link
Member Author

wxtim commented Oct 17, 2022

Modify ~/.ssh/config to break communications with remote host.

Surely this bit's not relevant?

Strangely I couldn't reproduce the bug without this step - The instance of WorkflowStopped which wasn't being caught was coming the network interfaces code.

@wxtim wxtim force-pushed the 20221014T1456--HEAD--fix_Cylc_Scan_Traceback_Bug branch from 982590c to eb62529 Compare October 17, 2022 09:20
@wxtim wxtim added this to the cylc-8.0.3 milestone Oct 17, 2022
@wxtim wxtim self-assigned this Oct 17, 2022
@wxtim wxtim added bug Something is wrong :( small labels Oct 17, 2022
@wxtim wxtim force-pushed the 20221014T1456--HEAD--fix_Cylc_Scan_Traceback_Bug branch from eb62529 to ddb9f78 Compare October 17, 2022 09:22
@oliver-sanders oliver-sanders modified the milestones: cylc-8.0.3, cylc-8.0.4 Oct 17, 2022
MetRonnie
MetRonnie reviewed Oct 17, 2022
View reviewed changes
CHANGES.md Outdated Show resolved Hide resolved
@wxtim wxtim force-pushed the 20221014T1456--HEAD--fix_Cylc_Scan_Traceback_Bug branch from ddb9f78 to 5016692 Compare October 18, 2022 14:03
@wxtim wxtim requested a review from MetRonnie October 18, 2022 14:04
@wxtim wxtim force-pushed the 20221014T1456--HEAD--fix_Cylc_Scan_Traceback_Bug branch from 5016692 to 5493ec2 Compare October 18, 2022 14:05
@wxtim wxtim marked this pull request as draft October 18, 2022 14:17
@wxtim wxtim force-pushed the 20221014T1456--HEAD--fix_Cylc_Scan_Traceback_Bug branch from 5493ec2 to 10606f3 Compare October 19, 2022 09:04
@wxtim wxtim marked this pull request as ready for review October 19, 2022 09:04
@wxtim
Copy link
Member Author

wxtim commented Oct 19, 2022

Getting the integration tests to do what I want was a hassle, but I think that I've done it!

hjoliver
hjoliver approved these changes Oct 21, 2022
View reviewed changes
Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

Looks good. I'll commit a small tweak to the change log, for clarity, and merge it.

CHANGES.md Outdated Show resolved Hide resolved
@hjoliver hjoliver merged commit f25e596 into cylc:8.0.x Oct 21, 2022
@wxtim wxtim deleted the 20221014T1456--HEAD--fix_Cylc_Scan_Traceback_Bug branch October 21, 2022 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MetRonnie MetRonnie MetRonnie approved these changes

@hjoliver hjoliver hjoliver approved these changes

@oliver-sanders oliver-sanders Awaiting requested review from oliver-sanders

Assignees

@wxtim wxtim

Labels
bug Something is wrong :( small
Projects
None yet
Milestone
cylc-8.0.4
Development

Successfully merging this pull request may close these issues.
5 participants
@wxtim @dpmatthews @hjoliver @oliver-sanders @MetRonnie