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

Redis asyncio invalid type definition. #8242

Open
s3rius opened this issue Jul 5, 2022 · 12 comments
Open

Redis asyncio invalid type definition. #8242

s3rius opened this issue Jul 5, 2022 · 12 comments
Assignees
Labels
topic: redis Issues related to the redis third-party stubs

Comments

@s3rius
Copy link

s3rius commented Jul 5, 2022

I found a problem using types-redis library with new redis.asyncio module.

The problem is that mypy throws an error.

The code:

async def get_redis_connection(request: Request) -> AsyncGenerator[Redis, None]:

The error:

testotlp/services/redis/dependency.py:7: error: Missing type parameters for generic type "Redis"  [type-arg]
    async def get_redis_connection(request: Request) -> AsyncGenerator[Redis, None]:

I tried to fix it by providing an Any generic type.

But found another issue. Mypy detects no error for the following code:

async def get_redis_connection(request: Request) -> AsyncGenerator[Redis[Any], None]:

But I cant start my program because of this:

  File "/testmypy/services/redis/dependency.py", line 7, in <module>
    async def get_redis_connection(request: Request) -> AsyncGenerator[Redis[Any], None]:
  File "../lib/python3.9/typing.py", line 277, in inner
    return func(*args, **kwds)
  File "../lib/python3.9/typing.py", line 1003, in __class_getitem__
    _check_generic(cls, params, len(cls.__parameters__))
  File "../lib/python3.9/site-packages/typing_extensions.py", line 92, in _check_generic
    raise TypeError(f"{cls} is not a generic class")
TypeError: <class 'redis.asyncio.client.Redis'> is not a generic class

I guess it's a problem.

@s3rius
Copy link
Author

s3rius commented Jul 5, 2022

Answered here: #7597 (comment)

@s3rius s3rius closed this as completed Jul 5, 2022
@s3rius
Copy link
Author

s3rius commented Jul 5, 2022

I think this issue must be reopened, because FastAPI depends on type system and the redis-types lib can't be used with FastAPI.

For all folks, who use redis with FastAPI consider downgrading types-redis to 4.2.0, delete .mypy_cache and rerun all checks.
This version of types-redis haven't got type annotations for asyncio package, but mypy won't throw any error.

Or you can downgrade mypy to 0.910, it also doesn't give this error.

@s3rius s3rius reopened this Jul 5, 2022
@JelleZijlstra
Copy link
Member

The runtime TypeError can be worked around by quoting the annotation, either explicitly (AsyncGenerator["Redis[Any]", None]) or with from __future__ import annotations.

Is there any other remaining problem?

@s3rius
Copy link
Author

s3rius commented Jul 6, 2022

Yes. The problem is that FastAPI framework uses types to inject dependencies. Quoting "Redis[Any]" doesn't work.
Importing from __future__ import annotations also doesn't work.

Here is a minimum reproducible example:

from __future__ import annotations

from typing import Any, AsyncGenerator

from fastapi import Depends, FastAPI, Request
from redis.asyncio import ConnectionPool, Redis

pool = ConnectionPool.from_url("redis://localhost:6379/0")


async def get_redis_connection(request: Request) -> AsyncGenerator[Redis[Any], None]:
    redis_client: Redis[Any] = Redis(connection_pool=request.app.state.redis_pool)

    try:  # noqa: WPS501
        yield redis_client
    finally:
        await redis_client.close()


app = FastAPI()


@app.get("/")
async def handler(redis: Redis[Any] = Depends(get_redis_connection)) -> None:
    await redis.set("key", "val")

Dependencies:

fastapi==0.78.0
redis==4.3.4
types-redis==4.3.3

Mypy config from pyproject.toml:

[tool.mypy]
strict = true
ignore_missing_imports = true
allow_subclassing_any = true
allow_untyped_calls = true
pretty = true
show_error_codes = true
implicit_reexport = true
allow_untyped_decorators = true
warn_unused_ignores = false
warn_return_any = false
namespace_packages = true

If you run this file you'll get an error.

@srittau
Copy link
Collaborator

srittau commented Jul 6, 2022

In this particular case, fastapi doesn't seem to use the type stubs for runtime type checking, instead using the runtime classes. I'm not sure that is even easily possible. That said, this is a larger issue that needs to be discussed on typing-sig, but unfortunately it's out of scope to fix in typeshed.

@s3rius
Copy link
Author

s3rius commented Jul 6, 2022

But maybe, if I provide a request with Redis type without Generic in definition, it will work as expected. Or it would be incorrect?

@srittau
Copy link
Collaborator

srittau commented Jul 6, 2022

That should help and is the correct long-term solution anyway, as redis is moving towards including types themselves.

@s3rius
Copy link
Author

s3rius commented Jul 6, 2022

Ok. I'll keep this issue open to be able to link pull request to it.

Can you assign this issue to me?

@samuelcolvin
Copy link
Sponsor Contributor

There's another problem here - inheriting from Redis.

Clearly we can't do class ArqRedis('Redis[bytes]'):, but class ArqRedis(Redis): is giving a typing error.

Frankly, I think that having the Redis stub as generic and the runtime class as not generic is a mistake.

@JelleZijlstra
Copy link
Member

You can work around this with something like:

if TYPE_CHECKING:
    _RedisBase = Redis[bytes]
else:
    _RedisBase = Redis

class ArgRedis(_RedisBase): ...

Admittedly it's not pretty.

@samuelcolvin
Copy link
Sponsor Contributor

Thanks both, that's what I have python-arq/arq#331.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: redis Issues related to the redis third-party stubs
6 participants