[14.0][FIX] datamodel: env of datamodel instances is not refreshed

[fdegrave]

I got frequent errors psycopg2.OperationalError('Unable to use a closed cursor.'). Upon closer inspection, it seems that the env linked to a datamodel is set once (at the init) but does not contain the env of the current request. This PR aims at fixing that.

[lmignon]

@fdegrave Your issue seems to be introduced by a model instance shared between requests?! At least, it highlight that the env should not be implicitly make available to a datamodel class...

[fdegrave]

@fdegrave Your issue seems to be introduced by a model instance shared between requests?! At least, it highlight that the env should not be implicitly make available to a datamodel class...

No, I did nothing exotic like that. But indeed my fix is not adapted to the core issue, that I discovered after some more digging. I figured out that this issue was introduced by this commit. Since the __init__ of a Datamodel class is patched only once for a given registry, the env is never refreshed. In my case I can clearly trace that during the second request the env of the service is fresh (with a opened cursor) but the one of the datamodel instance is not (cr is closed).

More generally, I really don't dig the "patch the init" kind of thing -- I find that rather ugly. Why not do something simpler, like using a class attribute? When I do this, my issue is fixed:

Modify the __init__ of Datamodel like this:

    def __init__(self, context=None, partial=None, env=None, **kwargs):
        self._env = env or type(self)._env
        super().__init__(context=context, partial=partial, **kwargs)

And change the __getitem__ of DataModelFactory like this:

    def __getitem__(self, key):
        model = self.registry[key]
        model._env = self.env

        @classmethod
        def __get_schema_class__(cls, **kwargs):
            cls = cls.__schema_class__(**kwargs)
            cls._env = self.env
            return cls

        model.__get_schema_class__ = __get_schema_class__
        return model

Don't you find that cleaner? And it fixes my issue as well... If you agree with that, I can modify this PR to contain only that instead.

[lmignon]

Modify the __init__ of Datamodel like this:

    def __init__(self, context=None, partial=None, env=None, **kwargs):
        self._env = env or type(self)._env
        super().__init__(context=context, partial=partial, **kwargs)

And change the __getitem__ of DataModelFactory like this:

    def __getitem__(self, key):
        model = self.registry[key]
        model._env = self.env

        @classmethod
        def __get_schema_class__(cls, **kwargs):
            cls = cls.__schema_class__(**kwargs)
            cls._env = self.env
            return cls

        model.__get_schema_class__ = __get_schema_class__
        return model

Don't you find that cleaner? And it fixes my issue as well... If you agree with that, I can modify this PR to contain only that instead.

@fdegrave Yes it's cleaner. I agree that the current implementation is ugly and reflect a lack of thinking at first implementation. In pydantic (#218), I no more provide the env property and no more use a registry to get access to a class. You just have to import your model and use it as normal python class and behind the scene you always use the class aggregating all the extensions declared into installed addons...

I agree with your fix. But it breaks existing implementation and therefore we've to change the major version of this addon... 14.0.3.0.4 => 14.0.4.0.0 (will be done at merge no need to do it into the code)

[fdegrave]

Done. And I'll definitely start experimenting with your pydantic module as soon as I have some time to do so. I like the approach as well.

[lmignon]

/ocabot merge patch

[OCA-git-bot]

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-227-by-lmignon-bump-patch, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at 2fe8088. Thanks a lot for contributing to OCA. ❤️

[lmignon]

@fdegrave It would be great it you agree to become the maintainer of the 'datamodel' addon. You contributed a lot on this.

Regarding pydantic, I found some issue in the current implementation but theses should be fixed in the coming weeks (This new addon is mainly enveloped in my spare time at this moment)