-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
Tornado5 #9448
Tornado5 #9448
Conversation
Hello @zulip/server-dependencies members, this pull request was labeled with the "area: dependencies" label, so you may want to check it out! |
Is the python 3.5 test failure due to the changes here or was that just something that periodically fails? |
I haven't seen that one fail nondeterministically; let me try to reproduce. |
It looks like it's having a problem with installing this: sinwar/sockjs-tornado@b31696b @sinwar did you do anything with that branch recently? |
fbd10c9
to
0f22190
Compare
This uses forked repo of socket-tornado. Fixes zulip#8913.
Heads up @scottbelden, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
Closed in favor of |
Resolves #8913
Testing Plan: Ran
./tools/test-all
A few notes:
Asyncio's loop doesn't seem to have any public functions for seeing what all is running so in https://github.com/scottbelden/zulip/blob/346aa2e9d0f854e71cb8c01e0b3c080445f27ae7/tools/run-dev.py#L398 I just changed to to stop. It might be possible to use
asyncio.Task.all_tasks
to get this information?I moved a lot of the logic to the
__init__.py
file. If I used theioloop_logging.py
file then there was a test that was failing because ourPolicy
wasn't getting set. See below:AsyncIOLoop
. Their docs (https://github.com/tornadoweb/tornado/blob/branch5.0/tornado/platform/asyncio.py#L167-L237) seem to imply that referencing those classes is deprecated. Instead we use thePolicy
to make sure that when tornado callsasyncio.get_event_loop()
it returns our custom event loop. One way I verified this manually was to modify the tornado source attornado.platform.asyncio
to add this print statement:NOTE: If you have that print in and run the dev script, the first print will still be the standard Unix event loop rather than our custom one. This is because that script starts the ioloop without setting the policy: https://github.com/zulip/zulip/blob/master/tools/run-dev.py#L417-L422.
@timabbott Let me know if there are any changes you would like. Hopefully this all makes sense...