[MIG][module_auto_update] Backport from v10 by yajo · Pull Request #918 · OCA/server-tools

yajo · 2017-08-01T10:25:10Z

Backporting #882

yajo · 2017-08-01T12:13:26Z

Cool! all 💚

pedrobaeza · 2017-08-01T12:19:17Z

-                'sha1',
-                excluded_extensions=exclude,
-            )
+            try:


Don't do this, but assign dirhash = None in import, and check here if dirhash:, but this should be declared as external dependency, so if the code get this, it's because you make sure the lib to be installed.

This is here because get_module_path can return False if an addon is installed bus it not found on disk. Then, dirhash raises a TypeError because it expects a str.

OK, thanks for clarifying. Can you please put the same comment in code for later references?

It's already there, above the pass

Ouch, got it

pedrobaeza

The case we have mentioned in the Docker project (new dependency added to installed modules) should be treated also:

Check which modules need to be updated (already done).
Update module list (but removing the part in this method that call again to update).
Check dependencies of these modules.
If some of the dependencies are not installed, then marked them for being installed.

There's still the chance that your installation doesn't contain the new dependency, but at least we reduce the possibilities.

pedrobaeza · 2017-08-01T12:24:53Z

+
+.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas
+   :alt: Try me on Runbot
+   :target: https://runbot.odoo-community.org/runbot/149/10.0


pedrobaeza · 2017-08-01T12:25:53Z

+    'category': 'Extra Tools',
+    'website': 'https://odoo-community.org/',
+    'author': 'LasLabs, '
+              'Juan José Scarafía, '


Shouldn't this be Ingeniería AdHoc? cc @jjscarafia

pedrobaeza · 2017-08-01T12:52:33Z

+
+    @api.model
+    def update_list(self):
+        res = super(Module, self).update_list()


This should be removed, as it's totally anti-performance, specially on OCB, where installing or updating a module first trigger a module list update, which in case of very large installations will take a lot of time. You already have a method for updating pending modules (running cron job manually), so don't force it here.

I didn't actually investigate, but I think this could be some of the issue behind the deadlocks I mentioned

I tested in a production database and indeed running odoo --stop-after-init -u any_module reloads dir hashes for all installed addons. Let's move all of this to the wizard method (which is also the one called from cron and Tecnativa/doodba#64) and I'll forward-port the patches.

lasley · 2017-08-01T16:15:51Z

cc @hughesbm

yajo · 2017-08-02T08:52:41Z

The refactor goes in no-fixup commits to be able to cherry-pick it later for v10. It required to change some tests.

pedrobaeza · 2017-08-02T14:59:03Z

+                    'sha1',
+                    excluded_extensions=exclude,
+                )
+                _logger.debug("Computed %s for %s", r.checksum_dir, r.name)


Should we keep this message?

Nah I think we should move it to the fail case instead (although with fail verbiage instead)

lasley · 2017-08-02T15:57:09Z

+                _logger.debug("Computed %s for %s", r.checksum_dir, r.name)
+            except TypeError:
+                # Module path not found
+                pass


Never pass silently

* [IMP] module_auto_update: Create new module * Add checksum_dir and checksum_installed fields to ir.module.module * Add checksum_dir to compute current checksum of module directory in addons path * Add checksum_installed to store checksum of module directory when module was last installed or upgraded * Use checksumdir Python library to compute module directory sha1 hashes, ignoring pyc and pyo extensions * Extend update_list method to compare modules' checksum_dir and checksum_installed, then change state of modules with differing checksums to 'to upgrade' * Replace Apps/Updates menu item with menu item of same name, which updates apps list and displays tree view of ir.module.module records with state 'to upgrade' * Extend create and write methods to store computed checksum_dir as checksum_installed during module installation and upgrade, and set checksum_installed to False on uninstall * Use context to stop checksum_installed from being updated during upgrade/uninstall cancellation * Add cron job to periodically check for module upgrades by comparing checksums, then perform any available upgrades * Extend upgrade_module method (called by cron and 'Apply Scheduled Upgrades' menu item) to call update_list * Add post_init_hook to store checksum_installed of existing modules * Add test coverage * [FIX] module_auto_update: Fix test broken by changes * Use dummy module to test update_list method instead of module_auto_update

By removing the recomputation from `update_list` we get faster CLI module upgrades and it only performs the autoupdate when using the autoupdate wizard or cron.

yajo · 2017-08-03T08:03:12Z

Rebased, squashed for merge

rafaelbn

We are using it in production enviroments

Syncing from upstream OCA/server-tools (14.0)

yajo self-assigned this Aug 1, 2017

yajo added the needs review label Aug 1, 2017

yajo added this to the 9.0 milestone Aug 1, 2017

pedrobaeza reviewed Aug 1, 2017

View reviewed changes

pedrobaeza requested changes Aug 1, 2017

View reviewed changes

pedrobaeza approved these changes Aug 2, 2017

View reviewed changes

lasley approved these changes Aug 2, 2017

View reviewed changes

hughesbm and others added 6 commits August 3, 2017 10:02

OCA Transbot updated translations from Transifex

6e41b7b

[MIG][module_auto_update] Backport from v10

f4305bc

[REF][module_auto_update] Recompute dir hashes only when needed

5a2c146

By removing the recomputation from `update_list` we get faster CLI module upgrades and it only performs the autoupdate when using the autoupdate wizard or cron.

[FIX][module_auto_update] Pass tests if addon is in readonly directory

83a6a50

[FIX][module_auto_update] Set dependencies to upgrade

f0ed842

yajo force-pushed the 9.0-module_auto_update branch from bedb81d to f0ed842 Compare August 3, 2017 08:02

rafaelbn approved these changes Aug 3, 2017

View reviewed changes

pedrobaeza merged commit c0a0b9a into OCA:9.0 Aug 3, 2017

pedrobaeza deleted the 9.0-module_auto_update branch August 3, 2017 08:22

lasley mentioned this pull request Aug 8, 2017

[REF][module_auto_update] Forward port v9 improvements #928

Merged

SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024

Merge pull request OCA#918 from OCA/14.0

c22fc25

Syncing from upstream OCA/server-tools (14.0)

Uh oh!

Uh oh!

Conversation

yajo commented Aug 1, 2017

Uh oh!

yajo commented Aug 1, 2017

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!

pedrobaeza left a comment

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!

lasley commented Aug 1, 2017

Uh oh!

yajo commented Aug 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lasley Aug 2, 2017 • 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!

yajo commented Aug 3, 2017

Uh oh!

rafaelbn left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lasley Aug 2, 2017 •

edited

Loading