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

Create odmantic model from pydantic model #60

Open
erny opened this issue Nov 30, 2020 · 11 comments · May be fixed by #62
Open

Create odmantic model from pydantic model #60

erny opened this issue Nov 30, 2020 · 11 comments · May be fixed by #62
Labels
enhancement New feature or request

Comments

@erny
Copy link
Contributor

erny commented Nov 30, 2020

I'm considering separating interface classes and persistence classes. Interface classes are the ones used in OpenAPI, i.e. in request / responses. Persistence classes are used when storing or retrieving objects from the store.

I would like to reuse pydantic models to create odmantic models, e.g. schemas.py (interface classes)

from pydantic import BaseModel, Field

class Publisher(BaseModel):
    name: str
    founded: int = Field(ge=1440)
    location: Optional[str] = None

And in models.py (persistence classes):

from odmantic import AIOEngine, Model
from schemas import Publisher

engine = AIOEngine()

class PublisherDB(Model, Publisher):
    pass

Although the metaclass does it's work, it seems that something does go wrong, because:

from models import PublisherDB, engine

engine.find(PublisherDB)
In [3]: res = await engine.find(PublisherDB, {})
---------------------------------------------------------------------------
ValidationError                           Traceback (most recent call last)
~/.local/share/virtualenvs/registry-KQbdKffM/lib/python3.8/site-packages/odmantic/model.py in parse_doc(cls, raw_doc)
    559         try:
--> 560             instance = cls.parse_obj(obj)
    561         except ValidationError as e:

~/.local/share/virtualenvs/registry-KQbdKffM/lib/python3.8/site-packages/pydantic/main.cpython-38-x86_64-linux-gnu.so in pydantic.main.BaseModel.parse_obj()

~/.local/share/virtualenvs/registry-KQbdKffM/lib/python3.8/site-packages/odmantic/model.py in __init__(self, **data)
    475     def __init__(self, **data: Any):
--> 476         super().__init__(**data)
    477         object.__setattr__(self, "__fields_modified__", set(self.__odm_fields__.keys()))

~/.local/share/virtualenvs/registry-KQbdKffM/lib/python3.8/site-packages/pydantic/main.cpython-38-x86_64-linux-gnu.so in pydantic.main.BaseModel.__init__()

ValidationError: 2 validation errors for PublisherDB
name
  field required (type=value_error.missing)
founded
  field required (type=value_error.missing)

During handling of the above exception, another exception occurred:

DocumentParsingError                      Traceback (most recent call last)
<ipython-input-3-b4c923edf0b7> in <module>
----> 1 res = await engine.find(PublisherDB, {})

~/.local/share/virtualenvs/registry-KQbdKffM/lib/python3.8/site-packages/odmantic/engine.py in __await__(self)
     64         instances = []
     65         for raw_doc in raw_docs:
---> 66             instances.append(self._parse_document(raw_doc))
     67             yield
     68         self._results = instances

~/.local/share/virtualenvs/registry-KQbdKffM/lib/python3.8/site-packages/odmantic/engine.py in _parse_document(self, raw_doc)
     54 
     55     def _parse_document(self, raw_doc: Dict) -> ModelType:
---> 56         instance = self._model.parse_doc(raw_doc)
     57         object.__setattr__(instance, "__fields_modified__", set())
     58         return instance

~/.local/share/virtualenvs/registry-KQbdKffM/lib/python3.8/site-packages/odmantic/model.py in parse_doc(cls, raw_doc)
    560             instance = cls.parse_obj(obj)
    561         except ValidationError as e:
--> 562             raise DocumentParsingError(
    563                 errors=e.raw_errors,  # type: ignore
    564                 model=cls,

DocumentParsingError: 2 validation errors for PublisherDB
name
  field required (type=value_error.missing)
founded
  field required (type=value_error.missing)
(PublisherDB instance details: id=ObjectId('5fc4e4c38c1d0709f79f1684'))
@art049 art049 mentioned this issue Nov 30, 2020
@art049 art049 added the enhancement New feature or request label Nov 30, 2020
@art049
Copy link
Owner

art049 commented Nov 30, 2020

Unfortunately, this behavior is not currently not supported. A way around however would be to define the pydantic model from the odmantic one. As explained in the usage with pydantic section of the docs.
But maybe having this behavior implemented as well could help.
Actually, I'm pretty sure this could be implemented in the meantime as model inheritance and it wouldn't be a huge effort once the initial inheritance would be implemented.
Being able to inherit from pydantic classes might raise some issues about the field definition (primary key, mongo name) though.

@erny
Copy link
Contributor Author

erny commented Dec 1, 2020

@art049
I was able to tweak odmantic.model.ModelMetaclass.new a bit as experiment. Here goes en example:

from pydantic import BaseModel, Field

import odmantic

class Publisher(BaseModel):
    name: str
    founded: int = Field(ge=1440)
    location: Optional[str] = None

class BookPublisher(Publisher):
    address: str
    employees: int = Field(ge=1)


class PublisherDB(odmantic.Model):
    __base__ = Publisher


class BookPublisherDB(odmantic.Model):
    __base__ = BookPublisher

and here changes to odmantic.model.ModelMetaclass.new:

...
def get_annotations(type_: type) -> dict:
    """Get type annotations of class hierarchy"""
    d = {}
    if issubclass(type_, pydantic.BaseModel) and type_ != pydantic.BaseModel:
        for base in type_.__bases__:
            d.update(get_annotations(base))
        d.update(type_.__annotations__)
    return d


class ModelMetaclass(BaseModelMetaclass):
    @no_type_check
    def __new__(  # noqa C901
        ...
    ):
        base = namespace.get('__base__')
        if base:
            namespace['__annotations__'] = get_annotations(base)
superclass
        if namespace.get("__module__") != "odmantic.model" and namespace.get(
        ...
        cls = super().__new__(mcs, name, bases, namespace, **kwargs)
        if base:  
            cls.__fields__.update(base.__fields__)   # use original fields
            cls.__pydantic_model__ = base
        return cls

I did some simple tests but there might be a million of cases that don't work

@erny erny linked a pull request Dec 1, 2020 that will close this issue
@art049
Copy link
Owner

art049 commented Dec 1, 2020

Thanks @erny for the fix. Do you know if the annotations are still available in the odmantic models with this ?

@erny
Copy link
Contributor Author

erny commented Dec 2, 2020

@art049 It's not a fix, but a hack (or tweak). Annotations should still be available. But I saw that the tests fail. I didn't do a complete checkout and run the tests locally. I replaced fields with the original pydantic ones, but it seems that your unit test don't like it. I'll try to work on this at another moment soon. Regards.

@art049
Copy link
Owner

art049 commented Dec 6, 2020

@art049 It's not a fix, but a hack (or tweak). Annotations should still be available. But I saw that the tests fail. I didn't do a complete checkout and run the tests locally. I replaced fields with the original pydantic ones, but it seems that your unit test don't like it. I'll try to work on this at another moment soon. Regards.

Yes, the odmantic Field object are not actually inheriting from the pydantic ones. In the beginning I though it would help separating the concerns. But now i think it would actually be helpful to inherit from those directly.

@art049 art049 linked a pull request Dec 6, 2020 that will close this issue
@lsaint
Copy link

lsaint commented Dec 11, 2020

As a fan of Clean Architecture , I think this feature is essential. What's better is have another to_entity method to convert to
pydantic model again.

@mnieber
Copy link

mnieber commented Dec 17, 2020

Maybe a naive question (I know almost nothing about odmantic internals) but would it be possible to make odmantic work directly with pydantic.BaseModel, and not have Model at all? Model seems to be a kind of wrapper, and without a wrapper it would be more likely that odmantic plays nicely with other libraries based on pydantic.

The only instance-level data I see in Model is the id field, so it might work if:

  • class-level information such as annotations can be stored externally (e.g. in some kind of singleton)
  • you make it mandatory for the pydantic.BaseModel instance to already have the 'id' field

@erny
Copy link
Contributor Author

erny commented Dec 22, 2020

I have to say in favour of the current implementation after using ODMantic that the Domain Object Model and the Persistence Object Model are different and that this forces us to separate cleanly both layers.

While the Domain Object Model is used mainly in the Service Layer as input parameters and return values, the Persistence Object Model focuses on how to store and retrieve objects. Often they are different: most of the times the persistent objects have more attributes than the domain objects. But in basic CRUD, return values are often persistence model instances which are the same or very similar to the domain model instances.

The service layer must translate domain objects to persistent objects and viceversa. I use attrs = pydantic x.dict(exclude_unset=True) a lot and odmantics Model(**attrs) to create objects and [setattr(model, attr, value) for attr, value in attrs.items()] for partial updates. While this is not very optimal, it fulfills its job.

I also put odmantic models (models.py) into another file as domain objects (schemas.py) and the service layer (services.py) imports both (and does the translations), while the presentation layer, the FastAPI REST views, just imports schemas.py.

@mnieber
Copy link

mnieber commented Dec 22, 2020

I think I understand the attraction in having both the pydantic model and the omdantic model, because it gives you more options as an odmantic developer (more things you can hook into). But maybe compare this to the example of an ORM that maps dictionaries to a database. If such an ORM works directly with a Python dict then it will be more attractive to end-users than an ORM that requires you to use "special dicts". In any case, I just wanted to offer this as a suggestion, I have no idea if it's actually feasible.

@art049
Copy link
Owner

art049 commented Dec 22, 2020

Maybe a naive question (I know almost nothing about odmantic internals) but would it be possible to make odmantic work directly with pydantic.BaseModel, and not have Model at all? Model seems to be a kind of wrapper, and without a wrapper it would be more likely that odmantic plays nicely with other libraries based on pydantic.

No worries @mnieber 😃 ; actually using the odmantic.Model class is necessary. The class itself is not the most important part but the metaclass is. The metaclass allows ODMantic to properly build the model object: provide the Proxy objects enabling filtering through python comparison operators, validating fields definition, providing substitution types for native BSON types to be handled.

While the Domain Object Model is used mainly in the Service Layer as input parameters and return values, the Persistence Object Model focuses on how to store and retrieve objects. Often they are different: most of the times the persistent objects have more attributes than the domain objects. But in basic CRUD, return values are often persistence model instances which are the same or very similar to the domain model instances.

Totally agree @erny , I faced as well the situation where it's needed to convert the odmantic instance to a dict and to parse it again into another pydantic model.

I use attrs = pydantic x.dict(exclude_unset=True) a lot and odmantics Model(**attrs) to create object

I didn't try it yet but it's probably possible to use the pydantic.BaseModel.from_orm as it would probably reduce the verbosity of the conversion and make it more handy for the developpers. If you had the occasion to try it, I would be really happy to bring this in the documentation or to complete the missing elements to make it work.

As we previously discussed and from the POC you showed @erny , creating ODMantic models from Pydantic models should be possible, in order to be able to pring all the desired functionalities: customize key_name, primary_key and also being able to specify the ODMantic Config object.
This should give the Pydantic.BaseModel to ODMantic.Model translation ability.

On top of this, since most of the time the domain objects are simply having fewer fields than the persistence layer objects (ODMantic models), I think that having a method to create pydantic submodels from an ODMantic model would be really helpful when architectures are smaller. I think this could be handled while working on "views" that would provide access to projected mongo documents (i.e. we only fetch a subset of the fields from documents stored in the database).

[setattr(model, attr, value) for attr, value in attrs.items()] for partial updates. While this is not very optimal, it fulfills its job.

The partial updates will be made more simple soon by #39 which will add a new Model.update method allowing object partial modification from either a pydantic object or a dictionary.

@hasansezertasan
Copy link

Hello everyone 👋. Thanks for tjr great work 🙏.

Is there any improvement on this issue? I didn't try the code and read the entire conversation. It's 3 years old, so I wanted to ask. 🤓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants