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

Make Hiredis Optional #917

Closed
Closed
@HassanAbouelela

Description

@HassanAbouelela

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.

Activity

added a commit that references this issue on Mar 18, 2021
e42bf48
HassanAbouelela

HassanAbouelela commented on Mar 19, 2021

@HassanAbouelela
Author

That was quick, thank you 🤗

WisdomPill

WisdomPill commented on Jul 18, 2021

@WisdomPill

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

Andrew-Chen-Wang

Andrew-Chen-Wang commented on Jul 18, 2021

@Andrew-Chen-Wang
Collaborator

@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

WisdomPill commented on Jul 18, 2021

@WisdomPill

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

Andrew-Chen-Wang commented on Jul 18, 2021

@Andrew-Chen-Wang
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 join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @WisdomPill@HassanAbouelela@Andrew-Chen-Wang

        Issue actions

          Make Hiredis Optional · Issue #917 · aio-libs-abandoned/aioredis-py