redis / redis-om-python

Object mapping, and more, for Redis and Python
MIT License
1.06k stars 108 forks source link

JsonSchema can not be generated #601

Closed prutheus closed 2 months ago

prutheus commented 3 months ago

As I thought that the https://github.com/redis/redis-om-python/pull/597 resolves my issues, I found out that following still does not work (but worked with older pydantic/redis-om versions):

Dependencies:

fastapi = "^0.110.0"
redis-om = "^0.2.2"

Model definition:

class ItemConfig(redis_om.HashModel):
    """
    Item Config
    """

    id: str = redis_om.Field(index=True, primary_key=True)
    note: Optional[str]
    purchase_price: Optional[float]

    class Meta:
        """
        Meta
        """

        global_key_prefix = "pw"
        model_key_prefix = "ItemConfig"

FastAPI controller, using ItemConfig model as body parameter for POST endpoint:

@router.post("/config")
def set_item_config(item_config: ItemConfig) -> None:
    ...

Navigating to the generated OpenAPI docs (localhost:8080/docs), getting error:

pydantic.errors.PydanticInvalidForJsonSchema: Cannot generate a JsonSchema for core_schema.PlainValidatorFunctionSchema ({'type': 'with-info', 'function': <bound method BaseModel.validate of <class 'models.ItemConfig'>>})
For further information visit https://errors.pydantic.dev/2.4/u/invalid-for-json-schema

It seems that the redis-om is not properly working with latest pydantic versions, e.g. the ItemConfig.model_json_schema method is not defined (what is new way to get the json schema of a Model).

@slorello89 @banker wdyt?

slorello89 commented 3 months ago

The issue you are tripping over is that Redis OM Python hasn't been migrated over to pydantic v2 yet, we support it in that Redis OM Python and pydantic v2 can exist in the same project, but the validators for pydantic v2 will not work with Redis OM python (which is why you are seeing that error).

See (__compat)[https://github.com/redis/redis-om-python/blob/main/aredis_om/_compat.py#L6] - basically if it sees you have pydantic v2 installed, it uses the v1 shim instead. FastAPI does somewhat the opposite (if you are on pydantic 1.x it more or less substitutes the fastAPI library with one compatible with pydantic 1.x)

We will need to make sure we actually support v2, but that looks like it will be a fairly significant change, so at the moment you can support FastAPI 0.100.0+ by specifying a pre-v2 pydantic version: pydantic = "<2.0.0", if that's an option for you.

prutheus commented 3 months ago

@slorello89 , thanks, but another question is, with the ItemConfig model from my issue description, FastAPI generates this json schema now:

{
  "pk": "string",
  "id": "string",
  "note": "string",
  "purchase_price": 0
}

How to get rid of additional pk field, since I marked id as primary_key already?

slorello89 commented 3 months ago

pk is part of the RedisModel, It looks like it just doesn't have a proper value if you have a separate pk defined in your model

prutheus commented 3 months ago

@slorello89 I would like to use a custom field as primary key, as you see in my initial post.

class ItemConfig(redis_om.HashModel):
    """
    Item Config
    """

    id: str = redis_om.Field(index=True, primary_key=True)
    note: Optional[str]
    purchase_price: Optional[float]

Id field should - and also works as - primary key here. However, the model json definition still contains the pk field, what is not necessary anymore.

Another solution could be to use the pk field but alias it with id:

[...]
    pk: str = redis_om.Field(index=True, primary_key=True, alias="id")

However, here I get the error:

NameError: Field name "pk" shadows a BaseModel attribute; use a different field name with "alias='pk'".

So what is the best way to use another name for the pk/primary key?

slorello89 commented 3 months ago

You can still specify that your id is the primary key, but pk is part of the RedisModel and thus inherited by any model's that you are declaring. If you have another pk and don't touch it though it just stays as an expression proxy and is never used, so it should be ignorable.