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

[IMP] util.modules: new hack to override load data mode #176

Closed

Conversation

HydrionBurst
Copy link
Contributor

@HydrionBurst HydrionBurst commented Dec 13, 2024

The new runtime option 'load_data_init' will be used for each package when loading package graph. It works like the previous package.init but more comprehensive and complete. Now if any directly or indirectly dependent module is configured 'load_data_init', the module will also be configured 'load_data_init'

odoo/odoo#189000

@robodoo
Copy link
Contributor

robodoo commented Dec 13, 2024

Pull request status dashboard

@HydrionBurst HydrionBurst force-pushed the master-core-refactor-graph2-cwg branch 2 times, most recently from 5b066ef to 7fe5c89 Compare January 16, 2025 08:55
@HydrionBurst HydrionBurst force-pushed the master-core-refactor-graph2-cwg branch from 1cff91a to 2a208aa Compare January 21, 2025 11:13
@KangOl
Copy link
Contributor

KangOl commented Jan 21, 2025

upgradeci retry with always only base

@HydrionBurst HydrionBurst force-pushed the master-core-refactor-graph2-cwg branch from 2355c77 to 6df6be9 Compare January 21, 2025 22:10
@HydrionBurst HydrionBurst force-pushed the master-core-refactor-graph2-cwg branch from 6df6be9 to 7137df7 Compare January 27, 2025 15:08
@HydrionBurst HydrionBurst force-pushed the master-core-refactor-graph2-cwg branch 2 times, most recently from 7de8679 to 1bbb64e Compare January 31, 2025 08:53
@@ -869,6 +869,11 @@ def force_upgrade_of_fresh_module(cr, module, init=True):
"""
Force the execution of upgrade scripts for a module that is being installed.

From saas~18.2
Odoo Standard Odoo allows to add module names to `registry._config_force_upgrade_scripts`
to force the execution of their upgrade scripts. `init=False` is not supported.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KangOl
I am not sure if we should support force_upgrade_of_fresh_module(cr, module, init=False) in the new strategy.
Could you help me check previously when we set init=False, did we really want the data and demo to be loaded in the update mode or we just didn't want the package.init to be propagated through dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

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

From memory, I thought it was only for avoiding propagation.
But we have cases where the init flag messed with the noupdate flag of the records:

@HydrionBurst HydrionBurst force-pushed the master-core-refactor-graph2-cwg branch 4 times, most recently from 9a91141 to f0179da Compare February 7, 2025 08:40
@HydrionBurst HydrionBurst force-pushed the master-core-refactor-graph2-cwg branch from f0179da to 896ec7a Compare February 7, 2025 12:49
Copy link
Member

@rco-odoo rco-odoo left a comment

Choose a reason for hiding this comment

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

Looks better (explicit) than the former hack (implicit).

@rco-odoo
Copy link
Member

@KangOl

Allow ``_force_upgrade_of_fresh_module`` in upgrade-util to force the execution
of upgrade script for new install modules
Unlike the old hack where
1. ``module_node.init`` can be changed by ``_force_upgrade_of_fresh_module``
   through changing the global ``config['init']``.
2. ``module_node.init`` was propagated only through one of the longest module
   dependency path.
The new hack uses a new explicit variable ``registry._force_upgrade_scripts``
without changing the global ``config``.

odoo/odoo#189000
@HydrionBurst HydrionBurst force-pushed the master-core-refactor-graph2-cwg branch from 896ec7a to 217ae25 Compare February 11, 2025 11:56
@rco-odoo
Copy link
Member

@aj-fuentes

Copy link
Contributor

@aj-fuentes aj-fuentes left a comment

Choose a reason for hiding this comment

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

LGTM

@aj-fuentes
Copy link
Contributor

@robodoo delegate=rco-odoo

@rco-odoo
Copy link
Member

@aj-fuentes thanks!

@rco-odoo
Copy link
Member

@robodoo r+

robodoo pushed a commit to odoo/odoo that referenced this pull request Feb 12, 2025
Before this commit
``load_module_graph`` didn't use ``update_module`` as a parameter. As a result
when loading registry in ``update_module=False`` mode, ``load_module_graph`` may
still upgrade 'to upgrade' modules in graph.

This could happen in multi-processes mode when
1. process 1 marks module A 'to upgrade' and is reloading the registry with
   ``update_module=True``, which will upgrade module A during loading modules.
2. process 2 is reloading the registry with ``update_module=False`` (e.g. the
   registry was invalidated before and has to be reloaded for the new request).
   During loading the registry, all 'installed', 'to remove', 'to upgrade' but
   not 'to install' modules will be loaded. And since module A is 'to upgrade',
   ``load_module_graph`` will also upgrade it.
The concurrent module upgrades will bring unexpected results.

After this commit
The `load_module_graph`` won't update modules when ``update_module=False``

Part-of: #189000
Related: odoo/upgrade-util#176
Signed-off-by: Raphael Collet <[email protected]>
robodoo pushed a commit to odoo/odoo that referenced this pull request Feb 12, 2025
In this commit replaces graph.py with module_graph.py which decouples the module
loading order from the loading.py. The new implementation introduces the
``ModuleNode`` class, which brings several improvements over the old ``Node``
class in graph.py:

1. No dynamic attributes:
   The ModuleNode class avoids dynamic attributes, making the code more
   predictable and maintainable.
2. Simplified update logic:
   * Removed ``node.init`` and ``node.update`` attributes, simplifying the
     update conditions. Now, loading.py relies solely on ``node.state`` to
     determine how to update modules.
   * The ``node.demo`` attribute is now exclusively used to reflect
     ``ir_module_module.demo`` in the database, indicating whether demo data has
     been successfully installed. This change necessitates a new strategy for
     setting up demo data during module loading. Based on all our use cases, we
     have removed the ``force_demo`` feature from ``Registry.new`` and
     ``load_modules`` and introduced a new feature, ``new_db_demo``, to
     determine whether demo data should be installed for a newly initialized
     database. Additionally, the ``load_module_graph`` function now includes an
     ``install_demo`` parameter to support the new feature.
3. Enhanced dependency management:
   The new implementation stores the complete dependency relations within the
   ModuleGraph, improving clarity and maintainability.
4. Optimized module serialization:
   The algorithm for module serialization has been simplified and optimized,
   reducing the time complexity from O(n²) to O(n log n)

Part-of: #189000
Related: odoo/upgrade-util#176
Signed-off-by: Raphael Collet <[email protected]>
robodoo pushed a commit to odoo/odoo that referenced this pull request Feb 12, 2025
Thanks to the new module_graph.py which can handle the ordering of "to update"
and "to upgrade" modules in ``update_module`` mode. The additional function
``load_marked_modules`` is no longer necessary and can be removed.

The old variables ``loaded_modules`` and ``processed_modules`` are already
consistent with ``registry._init_modules`` and ``registry.updated_modules``.
Since their states are maintained within the registry, there is no need for
``load_module_graph`` to return these variables explicitly.

Part-of: #189000
Related: odoo/upgrade-util#176
Signed-off-by: Raphael Collet <[email protected]>
robodoo pushed a commit to odoo/odoo that referenced this pull request Feb 12, 2025
remove config['demo'] since it is not used

Part-of: #189000
Related: odoo/upgrade-util#176
Signed-off-by: Raphael Collet <[email protected]>
robodoo pushed a commit to odoo/odoo that referenced this pull request Feb 12, 2025
robodoo pushed a commit to odoo/odoo that referenced this pull request Feb 12, 2025
Allow ``_force_upgrade_of_fresh_module`` in upgrade-util to force the execution
of upgrade script for new install modules
Unlike the old hack where
1. ``module_node.init`` can be changed by ``_force_upgrade_of_fresh_module``
   through changing the global ``config['init']``.
2. ``module_node.init`` was propagated only through one of the longest module
   dependency path.
The new hack uses a new explicit variable ``registry._force_upgrade_scripts``
without changing the global ``config``.

upgrade-util#176

closes #189000

Related: odoo/upgrade-util#176
Signed-off-by: Raphael Collet <[email protected]>
@robodoo robodoo closed this in 19d5972 Feb 12, 2025
@robodoo robodoo added the 18.2 label Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants