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

Fix to work with _api_request from Jupyterhub > 3 #255

Closed
wants to merge 2 commits into from

Conversation

rmalouf
Copy link

@rmalouf rmalouf commented Jan 6, 2023

After upgrading to Jupyterhub > 3, running batchspawner-singleuser produces the error:

/opt/conda/lib/python3.11/site-packages/batchspawner/singleuser.py:17: RuntimeWarning: coroutine 'HubAuth._api_request' was never awaited
  hub_auth._api_request(
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

and the singleuser server never makes contact with the jupyterhub server. Issues #233 and #253 may be related to this.

The source of the problem is that Jupyterhub refactored the routines in services.auth to allow them to work with async code. Connected to that, they switched from using requests to tornado.HTTPClient. This was supposed to be a transparent and non-breaking change, but batchspawner-singleuser cheats by calling an internal function (_api_request) and pays the price!

This PR just wraps the call to _api_request in asyncio.run() and replaces the json= arg with body=.

@welcome
Copy link

welcome bot commented Jan 6, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@dstndstn
Copy link

dstndstn commented Jan 6, 2023

Hi, Thank you for posting this. It looks like #250, #251 and this #255 are all trying to solve this same issue. Do you think you could merge your efforts? Would be wonderful to get this merged, since currently a simple "pip install" install fails. It looks like #251 has an attempt at backward-compatibility to earlier JHubs, so maybe that's preferable?
Thanks again!

@rmalouf
Copy link
Author

rmalouf commented Jan 7, 2023

Dang it! Not sure how I missed that, but yes, those are fixes for the same problem and #251 is more general. I'll just close this one.

@rmalouf rmalouf closed this Jan 7, 2023
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 this pull request may close these issues.

2 participants