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

Design good distribution extras support #2397

Closed
webknjaz opened this issue Oct 25, 2017 · 20 comments · Fixed by #3434
Closed

Design good distribution extras support #2397

webknjaz opened this issue Oct 25, 2017 · 20 comments · Fixed by #3434
Assignees

Comments

@webknjaz
Copy link
Member

Follow-up #2312 (comment): this issue is intended to encourage discussion about adding extras support.

There's several optional dependencies mentioned in docs, and they instruct users to do pip install {{dep_name}}.

Common practice is to add feature-flags allowing to depend on certain additional ability of the framework w/o having to install those dependencies manually.

For example, aiohttp could have speedups extra and then user's could've just pip install aiohttp[speedups] instead of pip install aiodns aiohttp cchardet.

Also, it is possible to use environment markers as extra names, in this case certain dependencies get installed automatically if the environment matches specific expression, which might be useful for windows-only crutches etc.

Please consider treating extras as a public API, because they are exposed to end-users.

/cc: @asvetlov

@webknjaz webknjaz self-assigned this Oct 25, 2017
@asvetlov
Copy link
Member

asvetlov commented Nov 8, 2017

I like the proposal, please go ahead

@asvetlov
Copy link
Member

Can we close it?
#3412 is merged.

@webknjaz
Copy link
Member Author

Let's keep it for discussion and close upon final edits. It's a public API, we'll end up maintaining it as accepted, remember?

@asvetlov
Copy link
Member

Agree

@webknjaz
Copy link
Member Author

webknjaz commented Dec 1, 2018

charset-guessing-speedup, async-dns and brotli-compression?

cc @WouldYouKindly

@asvetlov
Copy link
Member

asvetlov commented Dec 1, 2018

Perhaps my type speed is not enough for pip install aiohttp[charset-guessing-speedup,async-dns,brotli-compression].

I'm starting to wonder if we need separate entry points for every feature. One point for installing everything is better maybe? If a user wants something specific -- he can learn from extra_requires what to install.

@WouldYouKindly
Copy link
Contributor

@webknjaz @asvetlov why not just speedups, async-dns, and brotli?

charset-guessing-speedup / speedups
Those who want speed either don't really care where it comes from, or are willing to look into setup.py and see exact libraries — so I see no need for such a descriptive name. If later you add more speedups, extras API would not need to change.

brotli — the word is almost always used for a compression algorithm, so we may omit that part.

@asvetlov
Copy link
Member

asvetlov commented Dec 2, 2018

To avoid the naming problem we can add only pip install aiohttp[all] for everything.
No extras for cchardet, aiodns, and brotli; at least for now.

@kxepal
Copy link
Member

kxepal commented Dec 2, 2018

I think it would be better to limit support of extras by two cases: all extras on and all extras off. Otherwise we'll be burried by exponential growing combinations of those which need to support.

Is there a case when you need just some specific ones?

@webknjaz
Copy link
Member Author

webknjaz commented Dec 6, 2018

👎 for all — it's too "pointing to nowhere", all is not descriptive.
👍 for speedups — communicates intention to the user better.

@webknjaz
Copy link
Member Author

webknjaz commented Dec 6, 2018

PR: #3434

@webknjaz
Copy link
Member Author

webknjaz commented Dec 6, 2018

@asvetlov @kxepal @WouldYouKindly question: should we also all uvloop there?

@WouldYouKindly
Copy link
Contributor

@webknjaz if you decide to do so, it would be nice to mention uvloop in readme (along with brotlipy). (:

@kxepal
Copy link
Member

kxepal commented Dec 6, 2018

If we're going to make perfomance focused aiohttp installation, uvloop should be used and supported on code side as well. Not use that this is right decision since there could be different event loops, but I don't know any except uvloop and tokyo.

@asvetlov
Copy link
Member

asvetlov commented Dec 7, 2018

uvloop should be used and supported on code side as well

Not sure if I understand the sentence

@webknjaz
Copy link
Member Author

webknjaz commented Dec 7, 2018

I guess he means that aiohttp should automatically activate it if it's installed. Otherwise adding it as an extra will do nothing (except it'll just install it)

@kxepal
Copy link
Member

kxepal commented Dec 7, 2018

@asvetlov exactly what @webknjaz said.

@webknjaz
Copy link
Member Author

webknjaz commented Dec 7, 2018

@kxepal this comment explain why it's bad: #3434 (comment)

@kxepal
Copy link
Member

kxepal commented Dec 7, 2018

Yeap, it turns complicated /:

@lock
Copy link

lock bot commented Dec 7, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Dec 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants