Skip to content

redis-py async pipelines not being awaited is an error #8324

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
bentheiii opened this issue Jul 18, 2022 · 5 comments
Closed

redis-py async pipelines not being awaited is an error #8324

bentheiii opened this issue Jul 18, 2022 · 5 comments
Labels
stubs: false positive Type checkers report false errors

Comments

@bentheiii
Copy link
Contributor

types-redis incorrectly marks async Pipeline commands as coroutines, when in truth they do not need to be awaited to work (and are not implemented as coroutines.

the following snippet

from redis.asyncio import Redis
async def foo(redis: Redis):
  pipeline = redis.pipeline()
  pipeline.get(...)
  pipeline.set(...)
  await pipeline.execute()

is functional according to redis-py, but fails mypy with the error error: Value of type "Coroutine[Any, Any, bool]" must be used

@srittau
Copy link
Collaborator

srittau commented Jul 18, 2022

This is basically the same issue we have with the non-async Pipeline class: It needs to override all redis command methods so that it doesn't inherit them from the base (async) Redis class. Any PRs to improve the situation are welcome.

@srittau srittau added stubs: false positive Type checkers report false errors size-medium labels Jul 18, 2022
@bentheiii
Copy link
Contributor Author

Hi @srittau, I'd be happy to submit a PR, but I think a solution must be agreed upon beforehand. AFAICT every command in Pipeline can return either the pipeline itself or a coroutine to the command result. This leaves with 3 options I can see:

  1. All commands return Pipeline | Coroutine[Any, Any, ActualResultType] this is technically correct, but it means that neither pipeline.get(...).set(...).execute() nor (await pipeline.get(...)).decode() will succeed.
  2. All commands return Any. This will solve our issues but obviously will not catch other errors.
  3. wait for AnyOf I guess.

@srittau
Copy link
Collaborator

srittau commented Jul 18, 2022

Just to clarify (I didn't check): asyncio.Pipeline can return either a coroutine or itself? How is it decided what is returned? In case we can't model this, we should indeed just return Any, unfortunate as that is.

@bentheiii
Copy link
Contributor Author

AFAICT (judging by the source code), asyncio.Pipeline will return itself for method pipelining, UNLESS it exists in a special mode called watching, which it can enter at any time by someone calling the watch method, or exited by calling the multi method.

pipeline = redis.pipeline()
pipeline.get(...) # will return pipeline
pipeline.set(...)  # will return pipeline

pipeline.watch(...)  # will return a coroutine
pipeline.get(...) # will return a coroutine
pipeline.set(...)  # will return a coroutine

pipeline.multi() # returns none
pipeline.get(...) # will return pipeline
pipeline.set(...)  # will return pipeline

I'll get started on an Any solution

@bentheiii
Copy link
Contributor Author

#8325

@bentheiii bentheiii changed the title redis-py async pipelines must not being awaited is an error redis-py async pipelines not being awaited is an error Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: false positive Type checkers report false errors
Projects
None yet
Development

No branches or pull requests

2 participants