pydantic / pydantic-extra-types

Extra Pydantic types.
MIT License
176 stars 47 forks source link

Added in a new object id field for mongodb #151

Open SkandaPrasad-S opened 6 months ago

SkandaPrasad-S commented 6 months ago

Hi everyone -> this is for issue #133

I have been able to add a new pydantic object id field but I am facing some issues with serialisation and testing it

The field is validated correctly when used like

class Something(BaseModel):
    object_id: ObjectIdField

But I am facing some problem in doing Something.model_json_schema(mode="serialization").

Can someone look through my code and help me see what is wrong or what I need to change?

Ale-Cas commented 6 months ago

Hi everyone -> this is for issue #133

I have been able to add a new pydantic object id field but I am facing some issues with serialisation and testing it

The field is validated correctly when used like

class Something(BaseModel):
    object_id: ObjectIdField

But I am facing some problem in doing Something.model_json_schema(mode="serialization").

Can someone look through my code and help me see what is wrong or what I need to change?

I think to solve this you might need to implement this method:

@classmethod
def __get_pydantic_json_schema__(
        cls, schema: core_schema.CoreSchema, handler: GetJsonSchemaHandler
    ) -> dict[str, Any]: ...

similar to how it is implemented for other types

Ale-Cas commented 5 months ago

Also can you rename the new files to snake case (mongo_object_id.py and test_mongo_object_id.py) to follow the convention of other file names?

macintacos commented 1 week ago

Apologies, just wanted to check before I rolled my own implementation - is there still interest in getting this across the line? If @SkandaPrasad-S doesn't have time to get to this I may take a crack at it.

SkandaPrasad-S commented 1 week ago

Apologies, just wanted to check before I rolled my own implementation - is there still interest in getting this across the line? If @SkandaPrasad-S doesn't have time to get to this I may take a crack at it.

Hey @macintacos - go ahead, I tried to fix this a lot but it did not work. Could you just let me have a dummy commit to your contribution?

Kludex commented 1 week ago

I think a lot of ppl would benefit from this, but you can actually open a PR on bson to add the pydantic related function. I have merge rights there.

But... The bson you want is from pymongo - Which I don't have merge rights.

SkandaPrasad-S commented 2 days ago

I really want to close this off. Can I get some help on the testing side? @sydney-runkle

SkandaPrasad-S commented 2 days ago

Apologies, just wanted to check before I rolled my own implementation - is there still interest in getting this across the line? If @SkandaPrasad-S doesn't have time to get to this I may take a crack at it.

Also we can brainstorm together to fix it if needed to, please let me know if you want to