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

[14.0][ADD] pydantic: allows pydantic use into Odoo #218

Closed
wants to merge 8 commits into from

Conversation

lmignon
Copy link
Contributor

@lmignon lmignon commented Nov 7, 2021

The marriage of the power of Pydantic and the modular approach of Odoo. And yes, it's possible to write modern python code within odoo 😏

Not yet fully tested in a project....

@lmignon lmignon force-pushed the 14.0-add-pydantic branch 3 times, most recently from c060822 to 245d6bb Compare November 22, 2021 14:43
@lmignon lmignon marked this pull request as ready for review November 22, 2021 16:27
@lmignon lmignon changed the title [14.0][ADD] pydantic: allows pydantic usage into Odoo [14.0][ADD] pydantic: allows pydantic use into Odoo Nov 22, 2021
Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the beginning of a revolution :) Fantastic stuff.

pydantic/README.rst Outdated Show resolved Hide resolved
pydantic/README.rst Outdated Show resolved Hide resolved
pydantic/README.rst Show resolved Hide resolved
pydantic/builder.py Outdated Show resolved Hide resolved
pydantic/context.py Outdated Show resolved Hide resolved
field = self._obj._fields[key]
if field.type in ["one2many", "many2many"]:
return list(res)
return res
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything needed to convert False to None here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a test

pydantic/readme/USAGE.rst Outdated Show resolved Hide resolved
@gurneyalex
Copy link
Member

Hello @lmignon I'm testing pydantic on 15.0 using python 3.9. I get an error when I try to define a model with a Literal attribute.

class Order(BaseModel):
    warehouse: Literal["store", "olva", "spidy"]
    # lots of things commented out

Crashes the initialisation of the module:

Traceback (most recent call last):
  File "/odoo/src/odoo/service/server.py", line 1246, in preload_registries
    registry = Registry.new(dbname, update_module=update_module)
  File "/odoo/src/odoo/modules/registry.py", line 87, in new
    odoo.modules.load_modules(registry, force_demo, status, update_module)
  File "/odoo/src/odoo/modules/loading.py", line 581, in load_modules
    model._register_hook()
  File "/odoo/external-src/rest-framework/pydantic/builder.py", line 50, in _register_hook
    self.build_registry(registry)
  File "/odoo/external-src/rest-framework/pydantic/builder.py", line 83, in build_registry
    registry.init_registry([m.name for m in graph])
  File "/odoo/external-src/rest-framework/pydantic/registry.py", line 221, in init_registry
    self.resolve_submodel_fields()
  File "/odoo/external-src/rest-framework/pydantic/registry.py", line 176, in resolve_submodel_fields
    cls._resolve_submodel_fields(registry=self)
  File "/odoo/external-src/rest-framework/pydantic/models.py", line 185, in _resolve_submodel_fields
    cls._resolve_submodel_field(field, registry)
  File "/odoo/external-src/rest-framework/pydantic/models.py", line 196, in _resolve_submodel_field
    if issubclass(field.type_, BaseModel):
  File "/odoo/external-src/rest-framework/pydantic/models.py", line 154, in __subclasscheck__
    return super().__subclasscheck__(subclass)
  File "/usr/lib/python3.9/abc.py", line 102, in __subclasscheck__
    return _abc_subclasscheck(cls, subclass)
TypeError: issubclass() arg 1 must be a class

Do you have an idea what I could be doing wrong here?

@lmignon
Copy link
Contributor Author

lmignon commented Dec 8, 2021 via email

@lmignon
Copy link
Contributor Author

lmignon commented Dec 8, 2021

@gurneyalex The issue is fixed into my last commit.

@gurneyalex
Copy link
Member

@lmignon another one that tricked me: I'm unable to call super() on a model which python-inherits from another model:

from odoo.addons.pydantic.models import BaseModel

class MyModel(BaseModel):
    value: int=2

    def method(self):
        return self.value

class DerivedModel(MyModel):  # no extends, I want two different models here
    value2: int=3

    def method(self):
        return super().method() + self.value2

when I call method() on DerivedModel, I get "TypeError: super(type, obj): obj must be an instance or subtype of type"

Am I missing something?

@lmignon
Copy link
Contributor Author

lmignon commented Dec 13, 2021

@gurneyalex I added a test for your UC but I'm not able to reproduce it.

@gurneyalex
Copy link
Member

@gurneyalex I added a test for your UC but I'm not able to reproduce it.

weird... I ended up calling the parent method in the old fashioned way ParentClass.method(self) which worked (but speek of modern Python 🤣 ) I'll try to come up with a minimal example.

@lmignon
Copy link
Contributor Author

lmignon commented Dec 20, 2021

@gurneyalex I added a test for your UC but I'm not able to reproduce it.

@gurneyalex I reproduced the problem this weekend. It does not appear to be on py36 but on py38. It requires a lot of changes to solve it but I have the solution. As soon as I have some time, I will make the changes.

weird... I ended up calling the parent method in the old fashioned way ParentClass.method(self) which worked (but speek of modern Python rofl ) I'll try to come up with a minimal example.

This way of doing bypass the expected class hierarchy build by odoo when resolving the dependency graph...

@lmignon
Copy link
Contributor Author

lmignon commented Feb 2, 2022

Closing this PR since this functionality is now supported by the extendable-pydantic python package and the odoo's addon extendable, You can take al look at OCA/event#247 adding the new addon event_rest_api

@lmignon lmignon closed this Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants