Skip to content
This repository has been archived by the owner on Feb 21, 2023. It is now read-only.

Make Hiredis Optional #917

Closed
HassanAbouelela opened this issue Mar 18, 2021 · 5 comments
Closed

Make Hiredis Optional #917

HassanAbouelela opened this issue Mar 18, 2021 · 5 comments

Comments

@HassanAbouelela
Copy link

Description

With the current setup, the program works without hiredis installed, but it still has hiredis as a mandatory requirement:
https://github.com/aio-libs/aioredis/blob/53d8103cd69668e4a7ab500a9576abab1b359dad/setup.py#L51

That being said, the current workaround for not installing hiredis is forcing pypi to not install deps. That does work sometimes, but there are several problems. I see issues like #663, and all the other issues it links to, but I don't believe they were truly resolved.

The main argument against making it optional is that aioredis brings strong improvements in terms of speed. That may be true, but I believe it makes more sense to expect people to put in the effort to make things work better, instead of blocking the people that do not want to use hiredis (see my use case below for why workarounds do not work).

The way I've seen other libraries recommend optional dependencies is to log a warning during runtime, but still allow the program to run. Fuzzywuzzy for example logs the following:

./fuzzywuzzy/fuzz.py:11: UserWarning: Using slow pure-python SequenceMatcher. Install python-Levenshtein to remove this warning
  warnings.warn('Using slow pure-python SequenceMatcher. Install python-Levenshtein to remove this warning')

What I'm getting at is there are ways to make it very clear to people that you recommend something, be that on your pypi page, your docs, runtime warnings, or otherwise. It is not very easy for people to avoid the requirements in some cases (see my use case).

I have other possible suggestions, see Alternatives section below.

My Use Case

There is no good way to specify that a program shouldn't install optional deps if you aren't directly installing from the command line. Things like requirements files, and packaging tools like pipenv and poetry do not have support for this, because the solution suggested is less of a workaround, and more of a hack.

I've been trying to upgrade a project that uses pipenv and docker containers to 3.9 for a while, but hiredis-py has been dormant for quite a while. The build has been passing for 3.9 for nearly 3 months, but up until a week ago, there had been no activity in regards to getting a 3.9 wheel published (it is still not published, but action is being taken). I don't blame the hiredis maintainers, I'm sure they have other things to focus on, but the fact of the matter is, python will keep releasing new versions, and hiredis/aioredis will keep lagging behind if that library doesn't become more active, or if aioredis finally makes it optional.

I think its clear which of those solutions is simpler.

Implementation

I won't spend much time describing this, since there is pushback to the idea itself. It shouldn't be too bad since the core of the library already supports not having hiredis installed, it'll just be a few changed lines, and some documentation.

Alternatives

There are other solutions if you still want to keep the main aioredis package unchanged. One idea would be to have a second package that doesn't have hiredis. No other changes, just a change in the package setup.

@HassanAbouelela
Copy link
Author

That was quick, thank you 🤗

@WisdomPill
Copy link

Can this change be published in perhaps a new 1.3.2 version with just hired is as optional?

@WisdomPill WisdomPill reopened this Jul 18, 2021
@Andrew-Chen-Wang
Copy link
Collaborator

Andrew-Chen-Wang commented Jul 18, 2021

@WisdomPill Major version 1 requires hiredis; major version 2 does not because the code is completely different.

Edit: just taking a look at previous discussions again as a refresher; I still stand by the opinion to not make it optional tbh. 2.0.0 may not have been released yet, but to continue using 1.X.X is pointless. If it didn't work in the past, then why try to get a new patch release? Stick with 2.0.0b1+ or switch to aredis.

@WisdomPill
Copy link

Will try, I have had many problems in the past with hiredis, so I would prefer to sacrifice some performance for stability.
I am using hiredis through django_channels so the only option would be to install aioredis version 2 and see what happens. Are there any known breaking changes with moving to version 2?

@Andrew-Chen-Wang
Copy link
Collaborator

@WisdomPill yes, the library is now an async port of redis-py and doesn't really bare much resemblance to the 1.3.1 version. Follow #930 for installation instructions, follow https://aioredis.readthedocs.io/en/latest/migration/ for migration instructions.

Not sure how django-channels' aioredis dependency relates; just pin your aioredis version to 2.0.0b1 and you won't have hiredis installed anymore.

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

No branches or pull requests

3 participants