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

Running the tests locally doesn't clean up kernels #231

Closed
jtpio opened this issue Jun 13, 2019 · 12 comments · Fixed by #242
Closed

Running the tests locally doesn't clean up kernels #231

jtpio opened this issue Jun 13, 2019 · 12 comments · Fixed by #242

Comments

@jtpio
Copy link
Member

jtpio commented Jun 13, 2019

Running the tests locally spins up a lot of kernels but the kernels are not properly terminated at the end of the run.

Steps

  1. Run the tests: python -m pytest
  2. List voila processes: ps auxf | grep voila
  3. Sample output:
jtp      19860  0.5  0.3 534676 58080 ?        Ssl  10:42   0:00 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila_emfi2uwh/kernel-ca0eea8b-ae74-4d1e-9663-531e98ee1c32.json
jtp      19874  0.5  0.3 534676 58120 ?        Ssl  10:42   0:00 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila_nyn7s5ca/kernel-ec998d36-9c87-4e62-ae00-abf8558f3655.json
jtp      19888  0.5  0.3 534676 58036 ?        Ssl  10:42   0:00 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila_3fhelo7d/kernel-e213cc11-3397-405b-8527-b3a8a1fe7c96.json
jtp      19902  0.5  0.3 534676 58088 ?        Ssl  10:42   0:00 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila_w08vytrr/kernel-14835642-0f4d-4757-a36e-1f4351f89663.json
jtp      19950  0.5  0.3 534420 58108 ?        Ssl  10:42   0:00 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila_550qo66o/kernel-0d2582a4-6000-42fd-932a-3d2f965d4695.json
jtp      19992  0.6  0.3 534676 58052 ?        Ssl  10:42   0:00 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila_18o_lt7l/kernel-fde143c2-2bb2-4b70-9479-f2f7604bf216.json
jtp      20006  0.6  0.3 534676 58096 ?        Ssl  10:42   0:00 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila_ohi7_wla/kernel-422f0318-116b-4236-8096-69cc60299979.json
jtp      20020  0.6  0.3 534676 58144 ?        Ssl  10:42   0:00 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila_57akz99i/kernel-2696431b-e7b0-40f6-9b48-ebaaef5d81ba.json
jtp      20063  0.6  0.3 534676 58212 ?        Ssl  10:42   0:01 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila__170s2tn/kernel-f5dab699-54b6-4b17-8bac-56f93bc5e418.json
jtp      20077  0.7  0.3 534676 58100 ?        Ssl  10:42   0:01 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila_q4xzwhxg/kernel-47ca538e-a570-472c-af4b-452674f5e3c9.json
jtp      20091  0.8  0.3 534676 58048 ?        Ssl  10:42   0:01 /home/jtp/miniconda/envs/voila/bin/python -m ipykernel_launcher -f /tmp/voila_3m5b5r1k/kernel-9998e9d4-d8b0-457b-ad28-0de02ce41932.json

Maybe something like kernel_manager.shutdown_all() could be used to shut down all the kernels when the tests are over:

https://github.com/QuantStack/voila/blob/57182a943c18813d13f295cf11224301f6757f4f/voila/app.py#L466

@martinRenou
Copy link
Member

Isn't it annoying that it would shutdown all the kernels? Not only the ones that were spawned by the tests?

@jtpio
Copy link
Member Author

jtpio commented Jun 13, 2019

Indeed. Ideally it should only shut down the kernels spawned during the tests.

@jtpio
Copy link
Member Author

jtpio commented Jun 13, 2019

But this shutdown_all method is for a particular kernel_manager instance.

@martinRenou
Copy link
Member

It might be difficult to keep track over which kernels were spawned by voila though. I see that the kernel config files are in /tmp/voila_*

@martinRenou
Copy link
Member

But this shutdown_all method is for a particular kernel_manager instance

Oh ok cool!

@martinRenou
Copy link
Member

Actually, isn't it an issue of pytest-tornado? Shouldn't it stop all ioloops when tests are done?

@martinRenou
Copy link
Member

I tried something like that in the conftest.py:

@pytest.fixture(autouse=True, scope='module')
def shutdown_kernels():
    # Setup
    yield
    # Teardown
    current = tornado.ioloop.IOLoop.current(False)
    if current:
        current.stop()

But it breaks tests/app/template_cli_test.py and tests/app/template_custom_test.py with an:

tornado.httpclient.HTTPClientError: HTTP 500: Internal Server Error

other tests are passing. I'll dig a bit more tomorrow.

@martinRenou
Copy link
Member

I also wasn't sure if ioloops are started per function, per module or per session by pytest-tornado.

@maartenbreddels
Copy link
Member

This is because the custom Voila class https://github.com/QuantStack/voila/blob/6a1d9793b769ff3aad929b0497b9dcd4b6f41569/tests/app/conftest.py#L10 overrides listen(). We could move the shutdown of kernels from Voila.listen to Voila.start maybe?

@jtpio
Copy link
Member Author

jtpio commented Jun 13, 2019

Or move the self.kernel_manager.shutdown_all() to a stop() method and call the stop method here?

https://github.com/QuantStack/voila/blob/6a1d9793b769ff3aad929b0497b9dcd4b6f41569/tests/app/conftest.py#L44

@martinRenou
Copy link
Member

We could move the shutdown of kernels from Voila.listen to Voila.start maybe?

This does not seem to work. I feel like because the listen method from VoilaTest does nothing, it returns directly and shutdown everything.

Or move the self.kernel_manager.shutdown_all() to a stop() method and call the stop method here?

I tried, it makes the tests fail a bit randomly (I feel like one out of two is passing, but I'm not sure). They are failing with Internal Server Error

@jtpio
Copy link
Member Author

jtpio commented Jun 14, 2019

I tried, it makes the tests fail a bit randomly (I feel like one out of two is passing, but I'm not sure). They are failing with Internal Server Error

ok will try to reproduce that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants