Maintainership Moving Forward

[seandstewart]

State of the Repo

As a result of #820, I've taken on maintainership of this repository, with the main goal of my work being to resuscitate this library and bring it back up to speed with the reported bugs and open PRs.

I'm calling on the community for help!

I don't have a lot of time, but I didn't want to see this library die. I would please ask that if you stumble into this issue wondering if the library is dead or wishing to report an issue, please consider joining as a contributor!

Goals

• Update and add documentation for usage and for contributing
• Fix CI builds
• Migrate to GitHub Actions from Travis CI
• Address/merge outstanding PRs
• Triage existing tickets
• Resolve issues with supporting Python3.8+

This ticket will be updated as progress is made

[asvetlov]

Since Travis CI announced the very reduced plan for FOSS projects we need migrating to GitHub Actions probably.
I can help with it (as I did for many aio-libs projects already) and setup auto-deployment on PyPI by pushing git tag (again as we did for aio-libs in general).

[seandstewart]

@asvetlov that would be much appreciated. I do have some experience migrating to GHA, but I would like for this repo to follow the patterns already established in other aio-libs.

[Andrew-Chen-Wang]

@asvetlov I've implemented something similar when you mean auto-deployment with on-tag-release with actions: https://github.com/noripyt/django-cachalot/blob/master/.github/workflows/publish.yml

Although, I guess for more managed control, we can add something like:

if: github.actor == 'seandstewart'
    steps: etc.

so that it is only allowed if a certain user creates a release.

Edit: also, I think appveyor was just testing a Windows environment. We could probably remove appveyor as well in the GitHub actions.

[aviramha]

@seandstewart Awesome! I really didn't want the library to die. If you need any help you can assign issues to me and I'll take care of those.

[asvetlov]

@seandstewart sorry, I was busy this weekend with other projects.

Regarding the author check: I have no fundamental objections but I think the restriction for making a new release to writers only is not bad.
We are all adult people, I had never the case when somebody with write access pushed a tag accidentally to any aio-libs project.

P.S.
I still remember my commitment to setup GitHub CI. Just give me a few days please, maybe I can find time for it tomorrow.

[seandstewart]

Quick update here:

Outstanding Pull Requests is down from nearly 50 to under 20.

Along the way we've:

• Merged in fixes for a number of bugs.
• Dropped support for Python 3.5.
• Updated stale dependencies for testing and development.
• (mostly) Fixed the existing Travis CI.
• Made progress toward migrating to GitHub Actions.
• Migrated the code-base to Black for auto-formatting.

[seandstewart]

Just sending in another update:

I've been busy the past week, but I had some time today. Thanks to contributions from @asvetlov we've migrated over to GitHub Actions for our main CI.

I've also updated master to fix some tests which were broken from our recent flurry of merges and added a pre-commit configuration and make command make lint.

There is some outstanding work to be done in regards to CI:

• Migrate Windows tests from AppVeyor to GitHub Actions.
• Implement a CI job to automatically update pre-commit at a regular interval.

[Andrew-Chen-Wang]

@seandstewart One issue I'm seeing with Windows is that Redis doesn't work on Windows; redis supports using the subsystem linux only. I'm not really sure how many people are able to get redis working on Windows..., unless my time from windows has passed too long already; it's why I personally made a switch in OS.

[seandstewart]

@Andrew-Chen-Wang Yeah - it’s more so that we can be sure our library will work for developers who are using a Windows environment.

It’s not as critical yet because we don’t have any Cython or c extensions (yet) but it’s a good thing to do.

[seandstewart]

I seem to have broken our tests again when I added the latest Redis to the workflow. I've added #859 to triage the issue.

[aviramha]

@seandstewart Any ETA on releasing a new version?

[seandstewart]

Hey everyone -

Apologies for going dark for the last couple of weeks, work took over.

I've been digging into this library's core over the past week, and I'm beginning to think it may behoove us to take advantage of prior art (namely, redis-py). There are some design decisions in the core connection and parsing logic that may have made sense as a first pass, but add a lot of ambiguity to the whole codebase, namely:

1. Not buffering responses from redis into a byte-stream (as redis-py does). Instead we wrap our futures with deserialization futures, which is hard to debug and leads to ambiguous low-level asyncio errors. Buffering reads into a byte-stream means we can write more discernible and understandable code for our parsing, which now doesn't need to deal with futures directly.
2. Making the ConnectionPool and Connection objects interchangeable on a single attribute. For simple cases, this makes sense, but for many cases the ambiguity results in potentially bad or unexpected behavior. The codebase is littered with TODOs acknowledging areas where we need to go back and "pin" a connection to prevent unexpected behavior. A much better pattern here would be to have an explicit connection attribute and always explicitly aquire a connection if necessary.

Porting from redis-py also lowers the barrier to implementing new features as Redis advances, as we're now able to source potential solutions from another library. This makes maintainership far less burdensome moving forward, which means the risk of going stale is much lower.

I'll make an official issue for this proposal, and begin moving forward with a port, barring any strong backlash against this decision.

[Andrew-Chen-Wang]

@seandstewart Can I suggest that we push out a minor release before moving forward? There have been a lot of PRs merged without production/local testing their potential. I think it'd be a good idea to have people start testing the PR before making a huge port.

[seandstewart]

@Andrew-Chen-Wang that's a good call. I'll set up an alpha release of v2.0.0, since this will include a few breaking changes.

[abrookins]

Hi @seandstewart! I'm happy you jumped in to keep this library alive.

I work for Redis Labs, the maintainers of Redis. If long-term maintainership ends up being too much of a burden (or you already know you don't have the time), we're interested in taking this library on. We did the same recently with node-redis (https://github.com/NodeRedis/node-redis). Our goal is to make a core set of libraries more consistent and keep them up to date with the latest Redis features, and we have a dedicated engineering team that can make that happen.

[lovetoburnswhen]

Hi @seandstewart! I'm happy you jumped in to keep this library alive.

I work for Redis Labs, the maintainers of Redis. If long-term maintainership ends up being too much of a burden (or you already know you don't have the time), we're interested in taking this library on. We did the same recently with node-redis (https://github.com/NodeRedis/node-redis). Our goal is to make a core set of libraries more consistent and keep them up to date with the latest Redis features, and we have a dedicated engineering team that can make that happen.

Aw hell yeah

[seandstewart]

Hi @seandstewart! I'm happy you jumped in to keep this library alive.

I work for Redis Labs, the maintainers of Redis. If long-term maintainership ends up being too much of a burden (or you already know you don't have the time), we're interested in taking this library on. We did the same recently with node-redis (https://github.com/NodeRedis/node-redis). Our goal is to make a core set of libraries more consistent and keep them up to date with the latest Redis features, and we have a dedicated engineering team that can make that happen.

Hey @abrookins -

This is great news! We're currently working on a large port to get aioredis more in-line with the redis-py implementation (#891). This is close to merge and ready for a pre-release. If you're on board to take over maintainership during this time or after I'd be very grateful! My goal in all of this has been to ensure the library remains active and I can't think of a better place for the library to live than with the maintainers of Redis itself.

[seandstewart]

@asvetlov It looks like I need to be added as an admin to the Read the Docs project page: https://readthedocs.org/projects/aioredis/, I believe you should be able to accomplish this?

[abrookins]

@seandstewart Awesome! Unifying the APIs with redis-py will certainly make maintenance easier for me at least, as I'm more familiar with redis-py.

I'm happy to take over the repo now if you'd like. Most of us at Redis Labs are going to be busy until May working on RedisConf (April 20-21), but after that I'll have more time to dive in. Meanwhile, I could give you write permission so you can continue working on a pre-release, and then in May we could have a more formal hand-off? LMK what works for you -- I'm flexible.

[Andrew-Chen-Wang]

@seandstewart @abrookins I think in the meantime we can pin a new issue asking people to migrate to 2.0.0a1 since no one really sees alpha releases unless they watch the repo. Definitely shouldn't release 2.0.0 yet, but some form of publicizing this could be helpful to get feedback.

[yurymann]

Thanks again everyone.
What's preventing this from being released as a stable release? Is this just the lack of real-life testing and feedback or there's some known unfinished work? I'm keen to try migrating our production system because I need to improve/rewrite our reconnection logic and wouldn't like to waste time doing it with the old lib version. But will hold off (and maybe try to contribute if I find time) if there are known issues.

[seandstewart]

@yurymann -

The main issue is precisely what you've pinpointed. While we have a green test suite, this was a huge change in terms of LOC and hasn't been tested in any real-world scenario. I do want to do a pre-release cycle and pin an issue to flag passersby, but I'd like to hold off on a stable release until we've got some more confidence that it works as expected.

I'm currently blocked on releasing a pre-release package because I need administrator access to LGTM.com and ReadTheDocs so that I can adjust their integrations with this repository. I believe I'll need @asvetlov to make that happen.

I think in terms of API, we're stable and that won't be changing, so if you want to give a port a go and build off the master branch then by all means, go for it! Your work would be invaluable in terms of feedback on porting to the new implementation and isolating any issues we may have missed.

[yurymann]

Nice, thanks, I'll definitely give it a try when I run out of higher priority work.
@asvetlov, any help to keep Sean's momentum would be highly appreciated.

[abrookins]

I'll also work on testing the pre-release with a production service soon and report my findings in the forthcoming pinned GH issue. Exciting!

[Andrew-Chen-Wang]

A bunch of our CI tests are failing during make ci-test or basically when we're running unit tests. This is due to going over 15 minutes of no output. This doesn't occur in all of our matrices, but happens sporadically for no apparent reason: https://github.com/aio-libs/aioredis-py/runs/2175468093.

[seandstewart]

Hey everyone. I wanted to get on and let everyone know that I will not be as readily available for the next few weeks. There was a death in my family and I will be helping coordinate everything and won’t have much time to do anything else.

@abrookins @Andrew-Chen-Wang - I’ve made you both administrators. I leave it to y’all to decide whether you want to publish an alpha package before we’re able to fix the documentation. I’d also encourage getting a pinned issue for the v2 migration and starting a new project so we can track our progress towards a full release.

Best of luck everyone, I’ll be keeping an eye from afar and look forward to returning soon.

[abrookins]

I'm closing this issue. Thanks to @seandstewart Sean, aioredis-py managed to attract PRs from multiple community members, attracted high-quality repeat contributors, transitioned maintainership, and released version 2.0.0. Not too shabby! 🥳

[kevr]

Behavior was changed in 2.0.0 but not documented anywhere at all. Looking at #891, there's a checklist item for creating CHANGES/ files based on the changes in the pull. Those files are not there, even though it's checked as done. Additionally, I see that tasks for creating documentation are all checked off everywhere, but that does not seem to be the case at all.

If you're not going to provide documentation about it, pointing to behaviors in upstreams that you may not be able to control would be nice. Otherwise, we're just walking through the code changes to see what happened.

Additionally, I see some odd behavior with this implementation which was commented on by @Andrew-Chen-Wang on March 26th:

A bunch of our CI tests are failing during make ci-test or basically when we're running unit tests. This is due to going over 15 minutes of no output. This doesn't occur in all of our matrices, but happens sporadically for no apparent reason: https://github.com/aio-libs/aioredis-py/runs/2175468093.

I've encountered this issue with 2.0.0 as well, what's the dealio here?

Personally, I've seen it happen when Redis is unable to be contacted and ends up timing out for ~15 minutes, but, I've also seen behavior where when Redis is unable to be contacted, everything operates as it would as if it were connected, which is clearly impossible and it seems like no feedback is being provided for a failure to connect, as was previously by (IO|OS)Error.

[abrookins]

If you're seeing a potential problem with 2.0.0, which it sounds like you are, can you open an issue about that?

We have a pinned GitHub issue about the 2.0 migration which links to documentation on this topic. Those docs are also available/findable in the main docs site.

Did you read any of those?

[kevr]

If you're seeing a potential problem with 2.0.0, which it sounds like you are, can you open an issue about that?

We have a pinned GitHub issue about the 2.0 migration which links to documentation on this topic. Those docs are also available/findable in the main docs site.

Did you read any of those?

I did.

[Andrew-Chen-Wang]

@kevr just a note that when I noticed this, at the end of the 15 minutes, pytest/tox started running the first 10-30 tests but it was too late. Again, I'm unsure why. We've had weird connection errors that seem configuration dependent, but the only error I recall is if and only if the connection endpoint is not actually possible.

[seandstewart]

Hey @kevr -

Please see #930 for an in-depth discussion of the migration to this new implementation. Also note, the major changes in the API are notated in the documentation: Migrating to v2.0. That page, alongside our Getting Started page should get you started, after that, the API docs are up-to-date with the latest client implementation.

As for the CHANGES entries, those are consumed by towncrier and compiled into a changelog, so the directory should be empty after a release.

[yurymann]

In case people are interested: we migrated to the redis-py port v.2.0 in production back in September 2021 and have not seen any issues since then. We use it for high-frequency distributed task queues as well as caching on stand-alone Redis instances.

Thanks everyone involved for the great work!

[Andrew-Chen-Wang]

Hi all! I am in the works on creating the async module at redis-py. Please sit tight as that's where we'll be heading now.