Skip to content

Use optional package installs #298

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

Closed
wants to merge 4 commits into from
Closed

Conversation

almarklein
Copy link
Contributor

@almarklein almarklein commented Feb 10, 2019

This is #224 rebased to master. Closes #219.

I also pinned wsproto because uvicorn does not work for wsproto 0.13. Would probably be good to fix that (in another PR).

Suggestion: what about also adding [dev]. We could use that in the CI script, so we'd have all dependencies defined in a single place.

@almarklein
Copy link
Contributor Author

does not work for wsproto 0.13. Would probably be good to fix that (in another PR).

I'm looking into this.

@tomchristie
Copy link
Member

I'm not too sure how we want this to look.

Eg. A alternative that we could go for would be:

  • uvicorn - Just click.
  • uvicorn[standard] - click, uvloop, httptools, websockets.
  • uvicorn[pure] - click, h11, wsproto.

(And perhaps also uvicorn[full])

@tomchristie tomchristie changed the title Use optional package installs (rebased) Use optional package installs Feb 11, 2019
@almarklein
Copy link
Contributor Author

I agree that we should think this through. Especially as Uvicorn gets adopted by more frameworks. Most users won't interact with Uvicorn directly, but read the installation instructions on e.g. Responder, so the simpler we can make it, the better, I think.

The "story" that I'd prefer: Do pip install uvicorn to get a working Uvicorn on any platform. For more speed you can also install uvloop, httptools and websockets (if available on your platform). You can install these easily with pip install uvicorn[full].

In any way, I think that most users that run pip install uvicorn expect a working setup. Users that use httptools and websockets will have h11 and wsproto installed but unused. But since these are pure Python and not that big, I am personally not worried about that.

@tomchristie
Copy link
Member

Also reasonable:

  • uvicorn - Just click and h11.
  • uvicorn[standard] - click, h11, uvloop, httptools, websockets.
  • uvicorn[pure] - click, h11, wsproto.

@almarklein
Copy link
Contributor Author

Why exclude wsproto in the plain install?

As a Windows user, I feel a bit uncomfortable with the name "standard" for something that I cannot install :) (but I can get over it)

@almarklein
Copy link
Contributor Author

Anyway, it's your call :)

@tomchristie
Copy link
Member

Suppose we have an upstream library that wants to include the version of uvicorn most suited to the environment - are they able to do so using environment markers in the requirements?

What would the requirements look like for "Install uvicorn[pure] for windows and pypy, otherwise install uvicorn[full]"?

@almarklein
Copy link
Contributor Author

Suppose we have an upstream library that wants to include the version of uvicorn most suited to the environment

Right, that means we'd need to select the fast libs as supported by the platform. So back to the triage in setup.py as we had before this PR, though only for uvicorn[full].

After giving this some thought I'm not feeling too good about it, because I fear it might be fragile. E.g. we might miss something (e.g. what about IronPython), and uvloop wont compile on machines (or docker images) with less than 2 (or 4?) GB of RAM.

I'm starting to feel for something like this:

  • pip install uvicorn installs click, h11 and maybe wsproto. These always work, and will provide a fully functioning Uvicorn.
  • No environment markers.
  • Just tell users that uvloop, httptools and websockets can be installed for more performance. Upstream libraries should do the same. And up-upstream libraries too.

@tomchristie
Copy link
Member

So I'm trying to make a big push towards minimal dependency installs, so I'm quite keen on:

  • pip install uvicorn installs click.
  • pip install uvicorn[fast] installs click, uvloop, httptools, websockets.
  • pip install uvicorn[pure] installs click, h11, wsproto

Running uvicorn without an http implementation loudly errors with "Install either uvicorn[fast] or uvicorn[pure] - Please see link to helpful docs".

There's other configurations that uvicorn will start to become useful in too, eg. websockets-only, or lifespan-only (multi-workers running background tasks), and I'd like to see uvicorn be equally suitable for low-memory low-footprint deployments all the way up to all-the-bells-and-whistles.

@almarklein
Copy link
Contributor Author

Thanks for the clarification. I see your point.

The only remaining question I have now: on Windows, will pip install uvicorn[fast] try to install uvloop and httptools and fail, or do we install h11 instead? I think I am leaning towards the former, because we can't possibly keep track of all possibilities. If not on CPython + Unix, the user will need to use [pure] or manually select the libs that do work.

@tomchristie
Copy link
Member

on Windows, will pip install uvicorn[fast] try to install uvloop and httptools and fail

Yup pip install uvicorn[fast] would likely just fail there.

We'd be recommending uvicorn[pure] for Windows and PyPy, and uvicorn[fast] for CPython on Linux/Mac.

@almarklein
Copy link
Contributor Author

@tomchristie updated. Is this more or less how you'd like the installation instructions?

@carlwgeorge
Copy link
Contributor

For the Fedora package I'm working on, I can set up the dependencies as @tomchristie described.

Requires:       python3-click
Recommends:     python3-uvloop
Recommends:     python3-httptools
Recommends:     python3-websockets

All those would be installed by default, but a user can optionally disable installing the Recommends dependencies (or remove them after the fact).

@almarklein
Copy link
Contributor Author

Rebased (for changing wsproto req to 0.13).

@tomchristie
Copy link
Member

Happy with this - We'll just wait for a good point to make a major version bump is all.

@tomchristie
Copy link
Member

Coming back to this... I think we've got three different options here, for more minimal install options.

What do we think to each of these?...

Option 1:

$ pip install uvicorn  # click, h11, websockets
$ pip install uvicorn[fast]  # +httptools, +uvloop
$ pip install uvicorn[auto]  # +httptools, +uvloop (cpython, linux/mac)

Option 2:

We could omit websockets, and include nice loud messaging if we get websocket requests and neither wsproto or websockets is installed...

$ pip install uvicorn  # click, h11
$ pip install uvicorn[pure]  # +websockets
$ pip install uvicorn[fast]  # +websockets +httptools, +uvloop
$ pip install uvicorn[auto]  # +websockets (always) +httptools, +uvloop (cpython, linux/mac)

Option 3:

Super strict. Clear error messaging if you pip install uvicorn and haven't choosen a subtype install, or installed one of h11 or httptools seperately.

$ pip install uvicorn  # click
$ pip install uvicorn[pure]  # +websockets +h11
$ pip install uvicorn[fast]  # +websockets +httptools, +uvloop
$ pip install uvicorn[auto]  # +websockets (always) +h11 (pypy, windows) +httptools, +uvloop (cpython on linux/mac)

I think I quite like option 3 here, since it's really easy for us to point our users at pip install uvicorn[auto] as the default case. (And they'll get a clear error message if they've not done that) But they do also have more specific options if needed.

@gvbgduh
Copy link
Member

gvbgduh commented Oct 22, 2019

@tomchristie there's also the additional loop implementation currently available only for Windows - ProactorEventLoop, should be it also considered as a possible option or actually can be part of auto, but with explicit notice.

@tomchristie
Copy link
Member

Yes, that's a very good point. Is it supposed to have better performance characteristics?

@gvbgduh
Copy link
Member

gvbgduh commented Oct 22, 2019

I'm not sure precisely, but I think uvloop has issues with Windows support, but ProactorEventLoop should be definitely faster than native asyncio, so can work as a replacement.

@tomchristie
Copy link
Member

Oh, looks like it's the default in windows from 3.8 onwards https://twitter.com/VictorStinner/status/1109943702452088833 (So yeah, I'm perfectly okay with us essentially making it the default in 3.6, 3.7 too)

@gvbgduh
Copy link
Member

gvbgduh commented Oct 22, 2019

yes, uvloop seems to be not supported - MagicStack/uvloop#14

@tomchristie
Copy link
Member

@gvbgduh - Actually I was being slow here. Yes, ProactorEventLoop gets used by the "auto" loop choice, which is the default, but it's not relevant to this discussion since it doesn't require a package install.

@gvbgduh
Copy link
Member

gvbgduh commented Oct 22, 2019

yes, indeed!

@almarklein
Copy link
Contributor Author

... it's really easy for us to point our users at pip install uvicorn[auto] as the default case.

I'm not sure if I agree. Most users will actually use a higher level framework, and having pip install uvicorn not produce a working env will be confusing IMO. I'd therefore prefer something similar to your option 2.

BTW, is wsproto still an (optional) dependency? Since websockets is not pure-Python ...

@tomchristie
Copy link
Member

Most users will actually use a higher level framework, and having pip install uvicorn not produce a working env will be confusing IMO. I'd therefore prefer something similar to your option 2.

Noted. We could expect dependents to use uvicorn[auto] in their dependencies, but I'm kinda on the fence here.

It's not unreasonable for the base install to include h11, yeah.

BTW, is wsproto still an (optional) dependency? Since websockets is not pure-Python ...

wsproto is an optional yup. We support either that or websockets.

I'm not sure what the deal is with platform support for websockets - from searching the issue list I get the impression that it supports pypy just fine?

@almarklein
Copy link
Contributor Author

I'm not sure what the deal is with platform support for websockets - from searching the issue list I get the impression that it supports pypy just fine?

Hehe, it's that question mark at the end, right? Support for websockets seems really good across platforms, but the chance of an install fail are simply lower for a pure Python lib. Also, when using PyInstaller, non-pure Python libs can be problematic, though maybe that's besides the point. And, the fact that when I initially made this PR I needed to pin wsproto does not bode well for wsproto ;)

@tomchristie
Copy link
Member

tomchristie commented Oct 29, 2019

The "story" that I'd prefer: Do pip install uvicorn to get a working Uvicorn on any platform. For more speed you can also install uvloop, httptools and websockets (if available on your platform). You can install these easily with pip install uvicorn[full].

Yeah - probably right.

So having some time to think about this I reckon we can just go really simple...

$ pip install uvicorn  # A minimal install - just click and h11.
$ pip install uvicorn[default]  # An install complete with extras and platform dependencies - uvloop (cpython only), httptools (cpython only), websockets.

Our base install actually would be perfectly sufficient for most folks, and gives us a minimal install option for anyone wanting it, but pip install uvicorn[default] would give you the standard set of dependencies.

The following are also likely to be in our defaults list at some point:

  • colorama (windows only) - Would ensure that 0.10's coloured logging is also supported on windows.
  • watchdog - More efficient --reload support.
  • python-dotenv - To add support for --envfile .env.

I think either uvicorn[default] or uvicorn[defaults] is probably better than "full", because it's still only the platform dependent set of possible dependencies. And also it would only include "websockets", but not "wsproto" even though both are actually supported options.

Does that seem like a reasonable approach?

@almarklein
Copy link
Contributor Author

Does that seem like a reasonable approach?

Yes!

@almarklein
Copy link
Contributor Author

@tomchristie PR updated.

I'm feeling slightly uncomfortable about the word "default", which is the tag for extra installs, so not quite the default ;) Maybe pip install uvicorn[fast] (as in, as fast as we can get it on your platform)?

@tomchristie
Copy link
Member

I don't think fast is what we want. Eg. bumping out of the minimal install includes "websockets" (and other stuff in the future) which isn't about performance.

Perhaps this?...

pip install uvicorn[standard]

Any other naming ideas?

@almarklein
Copy link
Contributor Author

Maybe pip install uvicorn[full]?

@tomchristie
Copy link
Member

tomchristie commented Nov 4, 2019

Potentially, although eg. we don't want to include wsproto in there, even though it is supported, in which case "full" is a bit of a misnomer.

standard installs vs. minimal installs seems probably okay to me?

I guess we could have an "everything, including wsproto" option, giving us uvicorn / uvicorn[standard] / uvicorn[full] options, but I don't know if that's overkill?

@almarklein
Copy link
Contributor Author

but I don't know if that's overkill?

Yeah, that seems to overly complicate things. Updated to standard.

@euri10
Copy link
Member

euri10 commented May 4, 2020

this could be a good time to resurrect that PR since:

  1. we recently added extras_require after the watchgod addition
  2. there is a current PR proposal that would require another dependency, namely pyyaml

From the latest comments I gather here, there would be:

  1. pip install uvicorn # A minimal install
  • click
  • h11
  1. pip install uvicorn[standard] # An install complete with extras and platform dependencies

that leaves wsproto aside as @tomchristie last comment suggests.

if that's ok I can work on it or let you @almarklein rebase and finish ! :)

@almarklein
Copy link
Contributor Author

@euri10 I'm not able to work on this until next week, so please go ahead :)

@tomchristie
Copy link
Member

Let's close this in favour of #666. Thanks for your time on this @almarklein!

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.

Use optional package installs.
6 participants