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

Avoid runtime dependency on httpx unless testing is used #1826

Closed
Tronic opened this issue Mar 31, 2020 · 7 comments
Closed

Avoid runtime dependency on httpx unless testing is used #1826

Tronic opened this issue Mar 31, 2020 · 7 comments

Comments

@Tronic
Copy link
Member

Tronic commented Mar 31, 2020

Sanic only depends on httpx for testing but that actually becomes a hard dependency because sanic/__init__.py imports sanic.app which imports sanic.testing that then imports httpx. This is a problem because HTTPX frequently makes incompatible API changes and packages other than Sanic also depend on specific versions of it being installed.

On Sanic master branch one cannot even import sanic if there is httpx<0.10 or httpx>=0.12 installed, in contrast with Sanic 19.12LTS where one must have httpx<0.10.

Code could be restructured so that sanic.testing is only imported when needed. Possibly this means importing it inside app.test_client and app.asgi_client functions.

A related question is whether testing should be moved to a separate package or to pytest-sanic, to avoid installation of httpx by the main package. pip install won't refuse installs when there are conflicting requirements but wrong versions may end up installed.

Another alternative would be to use some other HTTP client in testing, one that has a more stable API and ideally one that would be fully asynchronous (encode/httpx#877).

Or if all of that seems too problematic, we could just do nothing and keep things the way they are, and try to keep up with httpx development. Care to share your thoughts?

@tnachen
Copy link

tnachen commented Apr 27, 2020

Also love to see httpx removed as a requirement, especially if it's only used in testing.

@ahopkins
Copy link
Member

The main problem is there is sort of a lack of decent http clients that can reach inside an ASGI compatible application and execute it without standing up the server. httpx follows on the heals of requests-async.

That being said, yes, I agree that it is annoying. I am in favor of doing something like this especially so that production builds can be as lean as possible.

I think this conversation does not stop with httpx, but should also include aiofiles and websockets.

Seeing as we already have one method for removing some dependencies at install time (SANIC_NO_UJSON and SANIC_NO_UVLOOP`), perhaps we go with a similar method. This could get messy.

The alternative would be to have various requirements relegated to extras:

$ pip install sanic[test,websockets]

To achieve this, we would probably need to add a constructor to Sanic to add those properties at runtime only if httpx was installed.

@Tronic
Copy link
Member Author

Tronic commented Apr 30, 2020

Agreed, httpx looks like the most feasible choice.

To achieve this, we would probably need to add a constructor to Sanic to add those properties at runtime only if httpx was installed.

Probably rather add functions to the class when the sanic module is imported. But wouldn't simple import statements inside test_client and asgi_client methods do? That shouldn't be too difficult to implement, and would not affect API docs and tools like dynamically creating these methods might.

The alternative would be to have various requirements relegated to extras:

$ pip install sanic[test,websockets]

I'm not a big fan of SANIC_NO_SOMETHING=YES install options; sanic[...] seems more reasonable.

@ashleysommer
Copy link
Member

During the Steering-Council meeting yesterday, we concluded a good idea would be to split out the sanic/testing.py code into a standalone package (sanic_testing?) and that will have the httpx dependency.

Then sanic can include sanic_testing as a dev_dependency or tests_dependency.

@ahopkins
Copy link
Member

ahopkins commented May 14, 2020

Closing in favor of #1850. See: https://github.com/huge-success/sanic-test

@ahopkins
Copy link
Member

@ashleysommer I pushed the repo before seeing your message. I like sanic_testing better to stay consistent with the existing module. With my next push I will rename it.

@ahopkins
Copy link
Member

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

No branches or pull requests

4 participants