[IMP] core: odoo.domains #99

kmagusiak · 2024-06-19T14:27:10Z

Add the new module in util.

robodoo · 2024-06-19T14:28:06Z

KangOl · 2024-06-24T09:28:25Z

Please refer to the odoo/enterprise/design-themes PR (if applicable) in the commit message.
It's needed by UpgradeCI to correctly build this PR with the corresponding branches.

If you don't want to have notifications on the related PR each time you force push, simply don't force push and add new fixup commits that will be squashed before the r+.

Note that UpgradeCI will also extract the info from the PR body. It is however better to have those references in the commit message itself.

_{This message is a canned response and thus may not be totally accurate.}

KangOl · 2024-06-24T09:31:00Z

upgradeci retry with always only product

src/util/domains.py

KangOl · 2024-09-16T11:35:22Z

upgradeci retry with always only helpdesk

kmagusiak · 2024-12-16T14:42:42Z

upgradeci retry with always only product

Adding domain management as an AST to the odoo.orm. We import the new package and fallback to expression.py, which will be deprecated in future releases. odoo/odoo#170009

aj-fuentes · 2024-12-16T14:47:39Z

src/util/domains.py

+    # the Domain factory normalized but also distributes operators during creation of domains
+
+    def normalize_domain(domain):
+        """Return a normalized version of the domain.


Why don't we implement this in terms of the new Domain factory?
return list(_dom.Domain(domain)) would this be enough?
If not, I still believe it's better to use the new implementation, it will bring better compatibility for future versions.

it is not strictly equivalent and some tests were failing.
Here is an example from an older build: https://runbot.odoo.com/runbot/build/64886163
One of the differences is stated above: list(Domain(xxx)) is distribute_not(normalize_domain(xxx)).

I suggest that we merge as-is, I would like to avoid mixing behaviour changes in this PR. I can implement a follow-up PR with the simplification.

That failure doesn't make sense 🤯 (irrelevantly of the current implementation) it's a test that should only run when going over 17.3. Why is it trying to upgrade from 17 to master, that's not valid. Only >=18 can be upgraded to master. Checking with runbot team...

I suggest that we merge as-is, and I can implement a follow-up PR with the simplification.

If you want to unblock it for the freeze, I'm OK. But do not forget to improve it later ;)

@aj-fuentes Added to my TODO list

Note that the last runbot run upgraded 18->master. Thus the tests that failed before, shouldn't anymore.

aj-fuentes · 2024-12-16T15:03:36Z

src/util/domains.py

+        return result
+
+    def normalize_leaf(leaf):
+        if not is_leaf(leaf):


If we allow in the linked PR in community to register optimizations that work accross all models we could do here:
list(domains.Domain("a","<>",1)._optimize(None))
which I can only seem to get to work with

list(domains.Domain("valid_field","<>",1)._optimize(self.env['base']))

I don't think you should optimize domains during the migration, it does more than simple rewrites.
That operator is replaced automatically for a long time and continues to work for now.

Anyways, same remark as above.

I agree, we don't need to optimize leaves. Only very simple details that are cross-model. That's why I asked if there was some sort of support for that. Anyway, we can keep this mocked normalize_leaf for now, but it should be aligned with current implementation. Namely it shouldn't warn if we don't warn for the equivalent in community. I'm not sure if we should do the replacing of operators either, seeing how that may affect the actual optimizations in the new implementation.

self.env['base'] is a particular model.
You need a different sentinel value for "any possible model".

You need a different sentinel value for "any possible model".

Indeed, to me it should be None. I was just hacking my way to get a feeling how this now works :)

aj-fuentes · 2024-12-16T15:15:06Z

src/util/domains.py

+        if operator == "<>":
+            operator = "!="
+        if isinstance(right, bool) and operator in ("in", "not in"):
+            _logger.warning("The domain term '%s' should use the '=' or '!=' operator.", ((left, original, right),))


Is this still true?

>>> list(domains.Domain("id","in",True)._optimize(self.env['base'])) [('id', 'in', [True])] >>> list(domains.Domain("active","in",True)._optimize(self.env['res.users'])) [('active', 'in', [True])] >>> list(domains.Domain("active","in",False)._optimize(self.env['res.users'])) [(0, '=', 1)]

The last output looks odd... I'd expect an error or [('active','in',[False])]/[('active','=',False)] as output.

If we no longer show any warning then why would we show one during the upgrade. If we now silently replace the leaf by something else, the warnings should be now more like This is translated to ... since Odoo 19.

I still think it's better to implement normalize_leaf using the new machinery at least for the cases where it doesn't do anything too involved.

The implementation is a copy of what is done today.
The last line comes from an optimization where in falsy_value results in a constant (because there are domains like ('a', '=', []) which do not mean = False, but in [] which is always False. More warnings and deprecations will come in the future on the community branch.

The implementation is a copy of what is done today.

That's what I understood. My point: we cannot keep for the future what's done today. It should be aligned. For merging the task this may be OK, but for the long term whatever we do here should be aligned with what's expected from the implementation in community.

Well spotted.

aj-fuentes

This is acceptable for a first iteration. I won't block it.

@robodoo delegate+

kmagusiak · 2024-12-16T16:56:55Z

@robodoo r+

robodoo · 2024-12-16T17:00:01Z

@kmagusiak linked pull request(s) odoo/documentation#10214 not ready. Linked PRs are not staged until all of them are ready.

New domains.py module to perform domain manipulation. This will replace the `odoo.osv.expression` module. The domain is now represented as an immutable abstract syntax tree; it is always normalized and allows to extend it more easily for things like adding optimizations. Today domains are manipulated as lists. This adds overhead when trying to understand the algorithms and you need to normalize manually. You may get undesired behaviours when you forget to normalize the domain or when you mutate the list. Concatenating two non-normalized lists is not the same as the and operator and if a field's search method mutates the domain that comes from a field's domain, it mutates it for the running instance. The newer API is fully backwards-compatible and looks like: domain = Domain(some_domain_from_user) domain &= Domain('id', '>', 5) # add a condition with an AND list(domain) # get the domain in the old-format task-3864512 closes #170009 Related: odoo/enterprise#65013 Related: odoo/documentation#10214 Related: odoo/upgrade-util#99 Signed-off-by: Raphael Collet <rco@odoo.com>

odoo/odoo#170009 closes #10214 Related: odoo/enterprise#65013 Related: odoo/upgrade-util#99 Signed-off-by: Antoine Vandevenne (anv) <anv@odoo.com>

kmagusiak force-pushed the master-odoo-domains-krma branch from 5d922c4 to 75995f8 Compare June 26, 2024 20:48

kmagusiak force-pushed the master-odoo-domains-krma branch from 75995f8 to d04730f Compare July 5, 2024 10:24

kmagusiak mentioned this pull request Jul 8, 2024

[IMP] core: odoo.domains odoo/odoo#170009

Closed

kmagusiak force-pushed the master-odoo-domains-krma branch 3 times, most recently from c1f14fd to cc14aca Compare July 12, 2024 05:47

kmagusiak force-pushed the master-odoo-domains-krma branch from cc14aca to 5fc5007 Compare July 18, 2024 15:27

KangOl reviewed Jul 18, 2024

View reviewed changes

src/util/domains.py Outdated Show resolved Hide resolved

kmagusiak force-pushed the master-odoo-domains-krma branch 4 times, most recently from 35f332a to ecb02d4 Compare July 19, 2024 06:21

kmagusiak marked this pull request as ready for review July 19, 2024 06:53

kmagusiak force-pushed the master-odoo-domains-krma branch from ecb02d4 to 9a28b72 Compare July 23, 2024 14:04

kmagusiak force-pushed the master-odoo-domains-krma branch 2 times, most recently from 67d7263 to a5e4a37 Compare August 4, 2024 06:09

kmagusiak force-pushed the master-odoo-domains-krma branch from a5e4a37 to 72636dd Compare August 19, 2024 15:45

kmagusiak force-pushed the master-odoo-domains-krma branch from 72636dd to 9bfd854 Compare September 9, 2024 09:51

kmagusiak force-pushed the master-odoo-domains-krma branch from 9bfd854 to 0871208 Compare September 16, 2024 10:46

kmagusiak force-pushed the master-odoo-domains-krma branch from 0871208 to 1bd88a5 Compare September 23, 2024 15:57

kmagusiak force-pushed the master-odoo-domains-krma branch from 1bd88a5 to 5fa525c Compare October 7, 2024 16:08

kmagusiak force-pushed the master-odoo-domains-krma branch from 5fa525c to 310a0c8 Compare October 23, 2024 09:08

kmagusiak force-pushed the master-odoo-domains-krma branch 3 times, most recently from b623c1e to ea0f06f Compare November 19, 2024 08:13

kmagusiak force-pushed the master-odoo-domains-krma branch from ea0f06f to aef38f5 Compare November 26, 2024 13:28

kmagusiak force-pushed the master-odoo-domains-krma branch 2 times, most recently from a3c8fdc to 2878002 Compare December 12, 2024 12:55

rco-odoo approved these changes Dec 16, 2024

View reviewed changes

kmagusiak requested a review from KangOl December 16, 2024 14:40

[IMP] core: odoo.domains

87d1ece

Adding domain management as an AST to the odoo.orm. We import the new package and fallback to expression.py, which will be deprecated in future releases. odoo/odoo#170009

kmagusiak force-pushed the master-odoo-domains-krma branch from 2878002 to 87d1ece Compare December 16, 2024 14:45

aj-fuentes reviewed Dec 16, 2024

View reviewed changes

kmagusiak requested a review from aj-fuentes December 16, 2024 15:34

aj-fuentes approved these changes Dec 16, 2024

View reviewed changes

robodoo pushed a commit to odoo/documentation that referenced this pull request Dec 17, 2024

[IMP] core: odoo.domain

87b489d

odoo/odoo#170009 closes #10214 Related: odoo/enterprise#65013 Related: odoo/upgrade-util#99 Signed-off-by: Antoine Vandevenne (anv) <anv@odoo.com>

robodoo closed this in 93e50b2 Dec 17, 2024

robodoo added the 18.1 label Dec 17, 2024

robodoo temporarily deployed to merge December 17, 2024 15:13 Inactive

kmagusiak deleted the master-odoo-domains-krma branch December 17, 2024 15:40

aj-fuentes mentioned this pull request Jan 22, 2025

[FIX] core: odoo.orm.domains #178

Closed

[IMP] core: odoo.domains #99

[IMP] core: odoo.domains #99

Uh oh!

Conversation

kmagusiak commented Jun 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robodoo commented Jun 19, 2024

Uh oh!

KangOl commented Jun 24, 2024

Uh oh!

KangOl commented Jun 24, 2024

Uh oh!

Uh oh!

KangOl commented Sep 16, 2024

Uh oh!

kmagusiak commented Dec 16, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmagusiak Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aj-fuentes left a comment

Choose a reason for hiding this comment

Uh oh!

kmagusiak commented Dec 16, 2024

Uh oh!

robodoo commented Dec 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kmagusiak commented Jun 19, 2024 •

edited

Loading

kmagusiak Dec 16, 2024 •

edited

Loading