pydantic-extra-types icon indicating copy to clipboard operation
pydantic-extra-types copied to clipboard

Added in a new object id field for mongodb

Open SkandaPrasad-S opened this issue 1 year ago • 7 comments

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?

SkandaPrasad-S avatar Mar 03 '24 06:03 SkandaPrasad-S

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 avatar Mar 04 '24 10:03 Ale-Cas

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?

Ale-Cas avatar Mar 06 '24 19:03 Ale-Cas

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.

macintacos avatar Aug 20 '24 18:08 macintacos

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?

SkandaPrasad-S avatar Aug 22 '24 04:08 SkandaPrasad-S

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.

Kludex avatar Aug 23 '24 08:08 Kludex

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

SkandaPrasad-S avatar Aug 30 '24 14:08 SkandaPrasad-S

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

SkandaPrasad-S avatar Aug 30 '24 14:08 SkandaPrasad-S

Would love to see this merged!

Todor97 avatar Jan 29 '25 15:01 Todor97

There are failing tests and conflicts in this PR, not sure when this is going to get picked up again but if you want I can open up a PR with my own implementation today based on the comments and suggestions above, should be ready by EOD. I'd also love this see this merged as I'm using heavily both pymongo and pydantic.

Ale-Cas avatar Jan 29 '25 15:01 Ale-Cas

There are failing tests and conflicts in this PR, not sure when this is going to get picked up again but if you want I can open up a PR with my own implementation today based on the comments and suggestions above, should be ready by EOD. I'd also love this see this merged as I'm using heavily both pymongo and pydantic.

Yes sounds good @Ale-Cas if you wanna work on this one 🙏🏻

yezz123 avatar Jan 29 '25 15:01 yezz123

@yezz123 @Kludex Please review: https://github.com/pydantic/pydantic-extra-types/pull/290

Ale-Cas avatar Jan 29 '25 19:01 Ale-Cas

Closed in favor of this #290

yezz123 avatar Jan 29 '25 19:01 yezz123