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

check if the fut was canncelled before setting result or exception #21

Merged
merged 8 commits into from Mar 29, 2019

Conversation

Projects
None yet
3 participants
@graingert
Copy link
Contributor

commented Mar 21, 2019

we mostly expect the fut to be cancelled if shutdown is happening, so this is very likely

@coveralls

This comment has been minimized.

Copy link

commented Mar 21, 2019

Coverage Status

Coverage increased (+0.09%) to 98.182% when pulling b93e032 on graingert:patch-2 into 981b470 on cjrh:master.

@cjrh

This comment has been minimized.

Copy link
Owner

commented Mar 26, 2019

@graingert Thanks for this, much better. Would you be able to add a test to hit this branch? https://coveralls.io/builds/22321261/source?filename=aiorun.py#L67

If not, that's ok, I'll try to add it when I have a moment. Perhaps this weekend.

@graingert graingert force-pushed the graingert:patch-2 branch from 368c394 to d2028bc Mar 27, 2019

check if the fut was cancelled before setting result or exception
we mostly expect the fut to be cancelled if shutdown is happening, so this is very likely

@graingert graingert force-pushed the graingert:patch-2 branch from d2028bc to 78e9806 Mar 27, 2019

graingert added some commits Mar 27, 2019

@graingert graingert force-pushed the graingert:patch-2 branch from 1dc1e52 to c78217a Mar 27, 2019

Show resolved Hide resolved aiorun.py
Show resolved Hide resolved aiorun.py Outdated
Show resolved Hide resolved tests/test_main.py
newloop()
run(main())
assert items
except KeyboardInterrupt:

This comment has been minimized.

Copy link
@cjrh

cjrh Mar 28, 2019

Owner

Under what circumstances does a KeyboardInterrupt get raised in the above code?

I'm guessing that you added this because maybe sometimes the shutdown doesn't actually occur and the loop never shuts down, and so you wanted a way to manually stop it...?

This comment has been minimized.

Copy link
@graingert

graingert Mar 28, 2019

Author Contributor

@cjrh I was having a problem where CI simply terminates: https://travis-ci.org/cjrh/aiorun/jobs/511961630#L244

@cjrh

This comment has been minimized.

Copy link
Owner

commented Mar 28, 2019

@graingert you should also add your name to the CONTRIBUTORS file in this PR.

graingert added some commits Mar 28, 2019

@graingert graingert force-pushed the graingert:patch-2 branch from 5b10f40 to c1edc1c Mar 28, 2019

@graingert

This comment has been minimized.

Copy link
Contributor Author

commented Mar 28, 2019

3.7+ seems to have some strange problem with keyboard interrupts propogating to the tests

Show resolved Hide resolved .travis.yml Outdated
travis improvements
explicit py3.8
explicit sudo: false
cache: pip

@graingert graingert force-pushed the graingert:patch-2 branch from 8808294 to b93e032 Mar 28, 2019

@cjrh

This comment has been minimized.

Copy link
Owner

commented Mar 29, 2019

@graingert looking good. I'll run the PR locally a few times on my machine at home this weekend just to look for any strangeness with the KeyboardInterrupt thingy, but I'm intending to merge. Are you ok for me to merge now, or are you still investigating anything?

@graingert

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

@cjrh cjrh merged commit 1cc93ee into cjrh:master Mar 29, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
coverage/coveralls Coverage increased (+0.09%) to 98.182%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.