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

tornado 5: PeriodicCallback loop arg will be removed #3034

Merged
merged 3 commits into from Nov 13, 2017

Conversation

Projects
None yet
2 participants
@minrk
Copy link
Member

minrk commented Nov 10, 2017

PCs are always run with the current eventloop, which is what the loop we have been passing explicitly already is.

Also fixed a case where we were closing ZMQStream and socket separately,
which may have been safe in the past, but can raise errors due to trying to unregister a closed socket with zmq.

minrk added some commits Nov 10, 2017

tornado 5: PeriodicCallback loop arg will be removed
PCs are always run with the current eventloop,
which is what the explicitly passed loop always is for us already
Don't double-close socket & stream
closing stream closes the socket

@minrk minrk added this to the 5.3 milestone Nov 10, 2017

@@ -452,7 +452,6 @@ def on_close(self):
# close the socket directly, don't wait for the stream

This comment has been minimized.

@takluyver

takluyver Nov 10, 2017

Member

This comment should probably be removed as misleading. And the assignment below is now unnecessary.

This comment has been minimized.

@minrk

minrk Nov 13, 2017

Author Member

Good catch, fixed.

@takluyver takluyver merged commit 1deb0ae into jupyter:master Nov 13, 2017

4 checks passed

codecov/patch Coverage not affected when comparing 9f72eb1...91078f5
Details
codecov/project 78.79% (+0.01%) compared to 9f72eb1
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

ryanlovett added a commit to ryanlovett/prob140hub that referenced this pull request Mar 8, 2018

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.