From 6f881a2bd14e68087f4916fca4106c1c5e3b7f38 Mon Sep 17 00:00:00 2001 From: Chris Busillo Date: Thu, 30 Jul 2026 02:11:53 -0400 Subject: [PATCH] fix: release module metadata locks before Odoo --- docker/scripts/run_odoo_data_workflows.py | 27 ++++--- docs/tooling/workspace-cli.md | 4 + tests/test_odoo_data_workflows.py | 93 ++++++++++++++++++++++- 3 files changed, 114 insertions(+), 10 deletions(-) diff --git a/docker/scripts/run_odoo_data_workflows.py b/docker/scripts/run_odoo_data_workflows.py index ab04f13..9737ce3 100644 --- a/docker/scripts/run_odoo_data_workflows.py +++ b/docker/scripts/run_odoo_data_workflows.py @@ -1011,7 +1011,10 @@ def _missing_manifest_install_queue_modules(self) -> list[str]: return sorted(module_name for module_name in queued_install_modules if module_name not in discovered_modules) def snapshot_module_states_before_openupgrade(self) -> None: - self._pre_openupgrade_module_states = self._module_states_by_name() + try: + self._pre_openupgrade_module_states = self._module_states_by_name() + finally: + self._reset_db_connection() def reconcile_missing_manifest_install_queue(self) -> None: unresolved_modules = self._missing_manifest_install_queue_modules() @@ -1890,13 +1893,16 @@ def _apply_module_updates( self.connect_to_db() rows: dict[str, str] = {} - with self.local.db_conn.cursor() as cur: - cur.execute( - "SELECT name, state FROM ir_module_module WHERE name = ANY(%s)", - (list(found),), - ) - for name, state in cur.fetchall(): - rows[name] = state + try: + with self.local.db_conn.cursor() as cur: + cur.execute( + "SELECT name, state FROM ir_module_module WHERE name = ANY(%s)", + (list(found),), + ) + for name, state in cur.fetchall(): + rows[name] = state + finally: + self._reset_db_connection() to_install = [name for name in found if name not in rows or rows.get(name) in ("uninstalled", "to remove")] to_update = list(found) if update_existing else [] @@ -2052,8 +2058,11 @@ def run_openupgrade(self) -> None: "Running OpenUpgrade with upgrade paths %s", ",".join(str(path) for path in scripts_paths), ) - self.run_command(" ".join(cmd_parts)) self._reset_db_connection() + try: + self.run_command(" ".join(cmd_parts)) + finally: + self._reset_db_connection() def _should_refresh_website_after_openupgrade(self) -> bool: target_version = (self.local.openupgrade_target_version or self.local.odoo_version or "").strip() diff --git a/docs/tooling/workspace-cli.md b/docs/tooling/workspace-cli.md index 4d728d8..fa77c8b 100644 --- a/docs/tooling/workspace-cli.md +++ b/docs/tooling/workspace-cli.md @@ -306,6 +306,10 @@ Notes - Devkit-managed startup and data workflow Odoo shell subprocesses prepend `/volumes/scripts` to `PYTHONPATH` so shipped runtime helpers remain importable from generated shell snippets. +- Data workflows close their module-state metadata transaction before launching + an Odoo install/update subprocess. This prevents the parent workflow from + retaining a read lock on `ir_module_module` while the child process performs + schema-changing module upgrades. - Release/deploy ownership for remote environments stays in `launchplane`, even when the same tenant manifest is used to anchor local runtime context. diff --git a/tests/test_odoo_data_workflows.py b/tests/test_odoo_data_workflows.py index a6ea892..c562d41 100644 --- a/tests/test_odoo_data_workflows.py +++ b/tests/test_odoo_data_workflows.py @@ -2,11 +2,12 @@ import importlib.util import os +import subprocess import sys import types import unittest from pathlib import Path -from unittest.mock import patch +from unittest.mock import MagicMock, patch def _load_data_workflows_module() -> types.ModuleType: @@ -19,6 +20,7 @@ def _load_data_workflows_module() -> types.ModuleType: original_sys_path = list(sys.path) psycopg2_module = types.ModuleType("psycopg2") + psycopg2_module.Error = Exception psycopg2_module.sql = types.SimpleNamespace(SQL=lambda value: value, Identifier=lambda value: value) psycopg2_extensions_module = types.ModuleType("psycopg2.extensions") psycopg2_extensions_module.connection = object @@ -154,6 +156,95 @@ def test_update_only_requires_configured_launchplane_payload_before_addon_update self.assertEqual(result, odoo_data_workflows.ExitCode.BOOTSTRAP_FAILED) update_addons.assert_not_called() + def test_module_update_releases_metadata_connection_before_odoo_command(self) -> None: + runner = odoo_data_workflows.OdooDataWorkflowRunner(self._local_settings(), upstream=None, env_file=None) + connection = MagicMock() + cursor = connection.cursor.return_value.__enter__.return_value + cursor.fetchall.return_value = [("cm_website", "installed")] + runner.local.db_conn = connection + + def assert_connection_released(_command: str) -> None: + self.assertIsNone(runner.local.db_conn) + connection.close.assert_called_once_with() + + with ( + patch.object(runner, "_resolve_addons_paths", return_value=(Path("/addons"),)), + patch.object(runner, "run_command", side_effect=assert_connection_released), + ): + runner._apply_module_updates( + ["cm_website"], + modules_source_label="test", + local_module_paths={"cm_website": Path("/addons/cm_website")}, + ) + + self.assertIsNone(runner.local.db_conn) + connection.close.assert_called_once_with() + + def test_module_update_keeps_connection_reset_when_odoo_command_fails(self) -> None: + runner = odoo_data_workflows.OdooDataWorkflowRunner(self._local_settings(), upstream=None, env_file=None) + connection = MagicMock() + cursor = connection.cursor.return_value.__enter__.return_value + cursor.fetchall.return_value = [("cm_website", "installed")] + runner.local.db_conn = connection + + def fail_after_connection_release(command: str) -> None: + self.assertIsNone(runner.local.db_conn) + raise subprocess.CalledProcessError(returncode=1, cmd=command) + + with ( + patch.object(runner, "_resolve_addons_paths", return_value=(Path("/addons"),)), + patch.object(runner, "run_command", side_effect=fail_after_connection_release), + self.assertRaises(odoo_data_workflows.OdooRestorerError), + ): + runner._apply_module_updates( + ["cm_website"], + modules_source_label="test", + local_module_paths={"cm_website": Path("/addons/cm_website")}, + ) + + self.assertIsNone(runner.local.db_conn) + connection.close.assert_called_once_with() + + def test_openupgrade_snapshot_releases_metadata_connection(self) -> None: + runner = odoo_data_workflows.OdooDataWorkflowRunner(self._local_settings(), upstream=None, env_file=None) + connection = MagicMock() + cursor = connection.cursor.return_value.__enter__.return_value + cursor.fetchall.return_value = [("base", "installed"), ("cm_website", "to upgrade")] + runner.local.db_conn = connection + + runner.snapshot_module_states_before_openupgrade() + + self.assertEqual( + runner._pre_openupgrade_module_states, + {"base": "installed", "cm_website": "to upgrade"}, + ) + self.assertIsNone(runner.local.db_conn) + connection.close.assert_called_once_with() + + def test_openupgrade_releases_cached_connection_before_odoo_command(self) -> None: + settings = self._local_settings() + settings.openupgrade_enabled = True + runner = odoo_data_workflows.OdooDataWorkflowRunner(settings, upstream=None, env_file=None) + connection = MagicMock() + runner.local.db_conn = connection + + def assert_connection_released(_command: str) -> None: + self.assertIsNone(runner.local.db_conn) + connection.close.assert_called_once_with() + + with ( + patch.object( + runner, + "_resolve_openupgrade_assets", + return_value=([Path("/addons/openupgrade_scripts/scripts")], Path("/addons/openupgrade_framework")), + ), + patch.object(runner, "run_command", side_effect=assert_connection_released), + ): + runner.run_openupgrade() + + self.assertIsNone(runner.local.db_conn) + connection.close.assert_called_once_with() + if __name__ == "__main__": unittest.main()