redis / redis-om-python

Object mapping, and more, for Redis and Python
MIT License
1.12k stars 112 forks source link

find method does not work #322

Closed johnson2427 closed 2 years ago

johnson2427 commented 2 years ago

What is the issue

I have created a FastAPI application, and I am trying to integrate aredis_om (but this fails with redis_om as well) with one of my rest endpoint modules.

I created a model called Item:

# app/models/redis/item.py
from aredis_om import Field, HashModel

from app.db.redis.session import redis_conn

class Item(HashModel):
    id: int = Field(index=True)
    name: str = Field(index=True)
    timestamp: float = Field(index=True)

    class Meta:
        database = redis_conn
# app/chemas/redis/item.py
from pydantic import BaseModel

class ItemCreate(BaseModel):
    id: int
    name: str
# app/db/redis/session.py
from aredis_om import get_redis_connection

from app.core.config import settings

redis_conn = get_redis_connection(
    url=f"redis://{settings.REDIS_HOST}:{settings.REDIS_PORT}",
    decode_responses=True
)
# app/api/api_v1/endpoints/redis_item.py
import time
from typing import Any, List, Optional

from fastapi import APIRouter, HTTPException
from aredis_om import NotFoundError

from app.models.redis.item import Item
from app.schemas.redis.item import ItemCreate

router = APIRouter()

@router.get("/", response_model=List[Item])
async def list_redis_items(name: Optional[str] = None) -> Any:
    items = []
    pks = [pk async for pk in await Item.all_pks()]
    for pk in pks:
        item = await Item.get(pk)
        if name is None:
            items.append(item)
        else:
            if item.name == name:
                items.append(item)
    return items

@router.post("/", response_model=Item)
async def post_redis_item(item: ItemCreate) -> Any:
    return await Item(id=item.id, name=item.name, timestamp=float(time.time())).save()

@router.get("/{id}", response_model=Item)
async def get_redis_item(id: int) -> Any:
    items = []
    pks = [pk async for pk in await Item.all_pks()]
    for pk in pks:
        item = await Item.get(pk)
        if item.id == id:
            return item

    raise HTTPException(status_code=404, detail=f"Item {id} not found")

@router.put("/{id}", response_model=Item)
async def update_redis_item(id: int, patch: Item) -> Any:
    try:
        item = await Item.get(id)
    except NotFoundError:
        raise HTTPException(status_code=404, detail=f"Item {id} not found")
    item.name = patch.name
    return await item.save()

As you can see in my endpoints file, I had to make a workaround to be able to pull the individual item, and to be able to get a list of items from Redis. My last endpoint, I believe is just wrong, I have to change id to a pk in order to get that item, so the last endpoint can be ignored.

My attempt for the first endpoint was this:

...
if name is None:
    items = await Item.find().all()
else:
    items = await Item.find(Item.name == name).all()
return items

When I hit the endpoint with the .find() method I received a traceback of:

INFO:     127.0.0.1:56128 - "GET /api/v1/redis_item/ HTTP/1.1" 500 Internal Server Error
ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/uvicorn/protocols/http/h11_impl.py", line 403, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/uvicorn/middleware/proxy_headers.py", line 78, in __call__
    return await self.app(scope, receive, send)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/fastapi/applications.py", line 269, in __call__
    await super().__call__(scope, receive, send)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/starlette/applications.py", line 124, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/starlette/middleware/errors.py", line 184, in __call__
    raise exc
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/starlette/middleware/errors.py", line 162, in __call__
    await self.app(scope, receive, _send)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/starlette/middleware/cors.py", line 84, in __call__
    await self.app(scope, receive, send)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/starlette/exceptions.py", line 93, in __call__
    raise exc
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/starlette/exceptions.py", line 82, in __call__
    await self.app(scope, receive, sender)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/fastapi/middleware/asyncexitstack.py", line 21, in __call__
    raise e
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/fastapi/middleware/asyncexitstack.py", line 18, in __call__
    await self.app(scope, receive, send)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/starlette/routing.py", line 670, in __call__
    await route.handle(scope, receive, send)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/starlette/routing.py", line 266, in handle
    await self.app(scope, receive, send)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/starlette/routing.py", line 65, in app
    response = await func(request)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/fastapi/routing.py", line 227, in app
    raw_response = await run_endpoint_function(
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/fastapi/routing.py", line 160, in run_endpoint_function
    return await dependant.call(**values)
  File "/home/user/ApeWorx/Kerkopes/kerkopes/backend/app/./app/api/api_v1/endpoints/redis_item.py", line 17, in list_redis_items
    return await Item.find().all()
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/aredis_om/model/model.py", line 760, in all
    return await query.execute()
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/aredis_om/model/model.py", line 725, in execute
    raw_result = await self.model.db().execute_command(*args)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/aioredis/client.py", line 1085, in execute_command
    return await self.parse_response(conn, command_name, **options)
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/aioredis/client.py", line 1101, in parse_response
    response = await connection.read_response()
  File "/home/user/.cache/pypoetry/virtualenvs/app-iUv8FE9o-py3.8/lib/python3.8/site-packages/aioredis/connection.py", line 919, in read_response
    raise response from None
aioredis.exceptions.ResponseError: unknown command `ft.search`, with args beginning with: `:app.models.redis.item.Item:index`, `*`, `LIMIT`, `0`, `10`, 

If you need more information from me, let me know! Thank you in advance.

wiseaidev commented 2 years ago

I think you forgot to run migrations first:

from fastapi import FastAPI

from redis_om import (
    Migrator
)

app = FastAPI(title=__name__)

@app.on_event("startup")
async def startup():
    await Migrator().run()

Update: Hey @johnson2427, i tried to run your code after running migrations, this time, i am getting the following error:

pydantic.error_wrappers.ValidationError: 1 validation error for Item
id
  field required (type=value_error.missing

Removed the id field, and the endpoint works fine:

Screenshot from 2022-08-08 13-47-35

It seems like id field is already defined in pydantic BaseModel, so try to rename it to something like id_

https://github.com/redis/redis-om-python/blob/dcd84e0ab2686a3c6ff7f6c6a43ec49f4b65655f/aredis_om/model/model.py#L1094-L1106

johnson2427 commented 2 years ago

@wiseaidev Thank you! This let's me narrow it down, but renaming that id field does not solve this issue. I still get pydantic.error_wrappers.ValidationError

wiseaidev commented 2 years ago

Hey @johnson2427, just giving it a default value seems to solve the issue:

id: int = Field(index=True, default=0)

I dunno how it works. MAGIC.

Update: I think there is a bug in the RedisModel implementation for integer fields. Also, Item.find(Item.id == id).all() won't work.

The only workaround, for now, is to rename the field to id_ of type str.

id_: str = Field(index=True)

In the meantime, I am trying to locate the bug.

wiseaidev commented 2 years ago

Y'all would laugh so hard if i should you the bug:

https://github.com/redis/redis-om-python/blob/dcd84e0ab2686a3c6ff7f6c6a43ec49f4b65655f/aredis_om/model/model.py#L1210-L1213

It seems like the developer was trying to implement something, didn't like it, then forgot about removing the code. LOl

wiseaidev commented 2 years ago

Time to run:

git blame aredis_om/model/model.py -L 1210,1213
0990c2e1 redis_developer/orm/model.py   (Andrew Brookins 2021-09-15 17:41:45 -0700 1210)             try:
d2fa4c58 redis_developer/model/model.py (Andrew Brookins 2021-10-20 13:01:46 -0700 1211)                 del fields["id"]
0990c2e1 redis_developer/orm/model.py   (Andrew Brookins 2021-09-15 17:41:45 -0700 1212)             except KeyError:
0990c2e1 redis_developer/orm/model.py   (Andrew Brookins 2021-09-15 17:41:45 -0700 1213)                 pass
(END)

Ayo, Andrew!

johnson2427 commented 2 years ago

hahahha, been there done that, no worries. I thought I was going insane. And I added the Migrator().run() and it kept failing, so instead of adding too many things at once, Thank you for the help!

sav-norem commented 2 years ago

@simonprickett seems like this issues has had a bug fix already merged in and can be closed