From 7296a0317904e9e9a81dde1102339c5ec367479b Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Tue, 18 Aug 2026 00:08:31 -0400 Subject: [PATCH] [FIX] migration: repair, replay, and know when to stop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The failure that stops an upgrade is almost always the same one — a COW copy left behind on the previous version — and the repair is known. So Enter now repairs, and a repair that actually changed something replays the command by itself. A default that acts must know when to stop, and here it must twice over. Repairing when there is nothing left to repair, then offering it again, loops without end: measured, « no COW copy has drifted » over and over. And a repair that does not help would replay the command forever. Three attempts, then the prompt says plainly that this one needs a developer. The turn counter is deliberately redundant with that logic: both guard the same failure, but the counter holds even if the logic is broken one day by accident. An endless loop in an unattended migration costs a night. Also: the smoke tool forced `ask=input` on its own prompt, which short-circuited auto-run — the question waited for a keystroke nobody was there to give. --- FR --- [FIX] migration : réparer, rejouer, et savoir s'arrêter L'échec qui arrête une migration est presque toujours le même — une copie COW restée sur la version d'avant — et la réparation est connue. Entrée répare donc, et une réparation qui a vraiment changé quelque chose rejoue la commande d'elle-même. Un défaut qui agit doit savoir s'arrêter, et ici deux fois plutôt qu'une. Réparer quand il n'y a plus rien à réparer, puis le reproposer, boucle sans fin : mesuré, « Aucune copie COW n'a dérivé », encore et encore. Et une réparation qui ne suffit pas rejouerait indéfiniment. Trois tentatives, puis l'invite dit qu'il faut un développeur. Le compteur de tours double volontairement cette logique : il tient même si elle est cassée un jour par mégarde. Assisted-by: Claude Opus 5 --- script/odoo/migration/smoke_public_url.py | 7 +- script/todo/todo_i18n.py | 20 ++ script/todo/todo_upgrade.py | 85 ++++++-- test/test_error_retry_loop.py | 229 ++++++++++++++++++++++ 4 files changed, 324 insertions(+), 17 deletions(-) create mode 100644 test/test_error_retry_loop.py diff --git a/script/odoo/migration/smoke_public_url.py b/script/odoo/migration/smoke_public_url.py index 3b7028a..0c7c22a 100755 --- a/script/odoo/migration/smoke_public_url.py +++ b/script/odoo/migration/smoke_public_url.py @@ -534,7 +534,7 @@ def run( boot=180, interactive=False, auto_apply=False, - ask=input, + ask=None, internal=True, internal_login="test", internal_password="test", @@ -612,6 +612,11 @@ def run( if code == 2: lst_done = [] else: + # `ask=None` et non `input` : c'est `prompt` qui sait quel défaut + # lui appartient, et qui pose alors la question par le lecteur + # temporisé. Forcer `input` ici court-circuitait le mode auto — + # mesuré, l'invite attendait indéfiniment une frappe pendant une + # migration lancée en auto-exécution. lst_done = prompt(database, lst_failure, lst_key, ask=ask) if not lst_done: return lst_url, lst_failure, lst_key, None, internal_report diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 6128d33..97f0b13 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -5462,6 +5462,26 @@ TRANSLATIONS = { "fr": "le lot entier", "en": "the whole batch", }, + "Still failing after": { + "fr": "Échoue toujours après", + "en": "Still failing after", + }, + "attempts: this one needs a developer.": { + "fr": "tentatives : celle-ci demande un développeur.", + "en": "attempts: this one needs a developer.", + }, + "Error detected. Choose, or ctrl+c to": { + "fr": "Erreur détectée. Choisissez, ou ctrl+c pour", + "en": "Error detected. Choose, or ctrl+c to", + }, + "stop": { + "fr": "arrêter", + "en": "stop", + }, + "Too many turns on this prompt: moving on.": { + "fr": "Trop de tours sur cette invite : on passe.", + "en": "Too many turns on this prompt: moving on.", + }, "Clean the database before testing the pages?": { "fr": "Nettoyer la base avant de tester les pages ?", "en": "Clean the database before testing the pages?", diff --git a/script/todo/todo_upgrade.py b/script/todo/todo_upgrade.py index d475978..1813421 100755 --- a/script/todo/todo_upgrade.py +++ b/script/todo/todo_upgrade.py @@ -761,6 +761,8 @@ class TodoUpgrade: return dct_kept AUTO_DELAY = 5 + MAX_ERROR_RETRY = 3 + MAX_ERROR_TURNS = 8 def prompt_auto_execute(self): """Proposer que les invites prennent leur défaut après un délai. @@ -2935,13 +2937,17 @@ class TodoUpgrade: dans un diff de mille lignes. On la recopie, on se trompe d'un caractère, et la commande ne fait rien sans le dire — une clé qui ne correspond à rien n'est pas une erreur pour l'outil. + + Rend True si quelque chose a VRAIMENT été réinitialisé. C'est ce qui + permet de rejouer la commande derrière : rejouer alors qu'on n'a + rien changé donnerait le même échec, indéfiniment. """ lst_key = self.stale_cow_keys(database_name) if not lst_key: print( f"✅ -> {t('No COW copy has drifted from its module view.')}" ) - return + return False print(f"\n✨ {t('Drifted COW copies')} :") for index, key in enumerate(lst_key, start=1): print(f" [{index}] {key}") @@ -2960,7 +2966,7 @@ class TodoUpgrade: # une sortie sans mot pour dire non serait une sortie sans issue. if not answer or answer == "n": print(f"ℹ -> {t('Kept. Nothing was reset.')}") - return + return False if answer == "a": lst_chosen = ["all"] else: @@ -2970,12 +2976,13 @@ class TodoUpgrade: lst_chosen.append(lst_key[int(part) - 1]) if not lst_chosen: print(f"⚠️ {t('Unknown choice, nothing was reset.')}") - return + return False args = " ".join(f"--reset {key}" for key in lst_chosen) - self.run_on_terminal( + status = self.run_on_terminal( f"{PYTHON_BIN} ./{os.path.join(PATH_MIGRATION_GLOBAL, 'reset_stale_cow_views.py')}" f" -d {database_name} {args} --apply" ) + return status == 0 def prompt_database_cleanup(self, database_name): """Proposer le nettoyage OCA avant d'interroger les pages. @@ -3481,6 +3488,7 @@ class TodoUpgrade: get_output=False, output_is_json=False, wait_at_error=True, + attempt=1, ): if output_is_json and not get_output: get_output = True @@ -3511,7 +3519,27 @@ class TodoUpgrade: # always sets one, but a silent None must not skip this prompt). if (status is None or status) and wait_at_error: database_name = self.database_from_command(cmd) + # « 3 » par défaut, car le motif d'échec le plus fréquent ici est + # une copie COW en retard : la réparer est presque toujours ce + # qu'on allait faire. Sans base nommée, les options 2 à 4 + # n'existent pas et le défaut redevient « continuer ». + defaut = "3" if database_name else "" + repare = False + tours = 0 while True: + # Une borne STRUCTURELLE, et non pas seulement la logique + # ci-dessous qui bascule le défaut. Les deux protections + # visent la même panne — une invite qui se repropose sans + # fin — mais celle-ci tient même si l'autre est cassée un + # jour par mégarde. Une boucle infinie dans une migration + # lancée sans surveillance coûte une nuit. + tours += 1 + if tours > self.MAX_ERROR_TURNS: + print( + f"🛑 {t('Too many turns on this prompt: moving on.')}" + ) + wait_status = "" + break print(f"[1] {t('to redo the command')}") if database_name: print( @@ -3525,8 +3553,10 @@ class TodoUpgrade: # bloquée sans que rien ne le signale. wait_status = ( self.ask( - f"💬 {t('Error detected, press enter to continue or')}" - f" ctrl+c {t('to stop')} : " + f"💬 {t('Error detected. Choose, or ctrl+c to')}" + f" {t('stop')}" + f" ({t('Enter')} = {defaut or t('continue')}) : ", + default=defaut, ) .strip() .lower() @@ -3543,7 +3573,17 @@ class TodoUpgrade: self.check_stale_cow_views(database_name) continue if wait_status == "3" and database_name: - self.prompt_reset_stale_cow_views(database_name) + repare = self.prompt_reset_stale_cow_views(database_name) + if repare: + # Quelque chose a changé : la commande mérite un + # nouvel essai, et c'est le seul cas où le rejeu + # est AUTOMATIQUE. + wait_status = "1" + break + # Rien à réinitialiser. Reproposer « 3 » ferait tourner + # en rond — mesuré : « Aucune copie COW n'a dérivé », + # encore et encore, sans fin. + defaut = "" continue if wait_status == "4" and database_name: # Sur le VRAI terminal : un plein écran refuse de @@ -3560,15 +3600,28 @@ class TodoUpgrade: break if wait_status == "1": - return self.todo_upgrade_execute( - cmd, - single_source_odoo=single_source_odoo, - new_env=new_env, - quiet=quiet, - get_output=get_output, - output_is_json=output_is_json, - wait_at_error=wait_at_error, - ) + # Le rejeu AUTOMATIQUE est borné ; celui qu'on demande à la + # main ne l'est pas. Sans cette borne, une réparation qui + # n'y suffit pas relancerait la commande indéfiniment — et + # une migration lancée en auto-exécution tournerait toute + # la nuit sur le même échec. + if repare and attempt >= self.MAX_ERROR_RETRY: + print( + f"🛑 {t('Still failing after')}" + f" {self.MAX_ERROR_RETRY}" + f" {t('attempts: this one needs a developer.')}" + ) + else: + return self.todo_upgrade_execute( + cmd, + single_source_odoo=single_source_odoo, + new_env=new_env, + quiet=quiet, + get_output=get_output, + output_is_json=output_is_json, + wait_at_error=wait_at_error, + attempt=attempt + 1, + ) if get_output: if output_is_json: diff --git a/test/test_error_retry_loop.py b/test/test_error_retry_loop.py new file mode 100644 index 0000000..296c012 --- /dev/null +++ b/test/test_error_retry_loop.py @@ -0,0 +1,229 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Réparer puis rejouer — et savoir s'arrêter. + +Le motif d'échec le plus fréquent d'une migration est une copie COW en +retard sur sa vue module : « Element cannot be located in parent +view ». La réparation est connue, et c'est presque toujours ce qu'on allait +faire. D'où « 3 » par défaut, et le rejeu automatique derrière. + +Mais un défaut qui agit doit savoir s'arrêter, et à deux titres : + +- réinitialiser quand il n'y a RIEN à réinitialiser puis reproposer la même + chose est une boucle sans fin. Vécu : « Aucune copie COW n'a dérivé », + encore et encore ; +- une réparation qui ne suffit pas relancerait la commande indéfiniment. + Trois tentatives, puis on rend la main : une migration lancée en + auto-exécution tournerait sinon toute la nuit sur le même échec. + +Ces tests comptent les tours. C'est la seule façon de prouver qu'une +boucle se termine. +""" + +import io +import os +import unittest +from contextlib import redirect_stdout + +REPO = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + +from script.todo import todo_i18n # noqa: E402 +from script.todo.todo_upgrade import TodoUpgrade # noqa: E402 + + +class Harness(unittest.TestCase): + """Un pilote dont la commande échoue toujours, et qu'on observe.""" + + def setUp(self): + self.addCleanup( + setattr, todo_i18n, "_current_lang", todo_i18n._current_lang + ) + todo_i18n._current_lang = "en" + + def upgrade(self, lst_answer=None, resets=None, echec=True): + """`resets` : ce que la réinitialisation rend, tour après tour.""" + obj = TodoUpgrade.__new__(TodoUpgrade) + obj.dct_progression = {} + obj.lst_command_executed = [] + obj.write_config = lambda: None + obj.database_from_command = lambda cmd: "db" + self.lst_run = [] + self.lst_reset = [] + + class FauxExecute: + def exec_command_live(_self, cmd, **kw): + self.lst_run.append(cmd) + # Toujours en échec : c'est le cas qu'on veut borner. + return (1 if echec else 0), cmd + + obj.execute = FauxExecute() + suite = iter(resets if resets is not None else []) + + def faux_reset(database): + self.lst_reset.append(database) + try: + return next(suite) + except StopIteration: + return False + + obj.prompt_reset_stale_cow_views = faux_reset + obj.check_stale_cow_views = lambda db: None + obj.run_on_terminal = lambda cmd: 0 + reponses = iter(lst_answer or []) + + def faux_ask(prompt, default=""): + try: + return next(reponses) + except StopIteration: + # Personne ne répond : c'est le mode auto, et c'est + # précisément là qu'une boucle sans fin se déclenche. + return default + + obj.ask = faux_ask + return obj + + def executer(self, obj): + out = io.StringIO() + with redirect_stdout(out): + obj.todo_upgrade_execute("./script/addons/update_addons_all.sh db") + return out.getvalue() + + +class TestRepairingThenReplaying(Harness): + def test_the_default_repairs(self): + # Sans réponse, on répare : c'est le geste qu'on allait faire. + obj = self.upgrade(resets=[True, False]) + self.executer(obj) + self.assertTrue(self.lst_reset) + + def test_a_successful_repair_replays_the_command(self): + obj = self.upgrade(resets=[True, False]) + self.executer(obj) + self.assertGreaterEqual(len(self.lst_run), 2) + + def test_a_repair_that_changed_nothing_does_NOT_replay(self): + # Rejouer sans avoir rien changé donnerait le même échec. + obj = self.upgrade(resets=[False]) + self.executer(obj) + self.assertEqual(len(self.lst_run), 1) + + +class TestItAlwaysStops(Harness): + """La propriété qui compte le plus : la boucle se termine.""" + + def test_nothing_to_reset_does_not_loop_forever(self): + # Vécu : « Aucune copie COW n'a dérivé », reproposé sans fin parce + # que le défaut restait « 3 ». + obj = self.upgrade(resets=[False, False, False, False, False]) + self.executer(obj) + self.assertLessEqual(len(self.lst_reset), 2) + + def test_a_repair_that_never_helps_stops_after_three(self): + obj = self.upgrade(resets=[True] * 20) + self.executer(obj) + self.assertEqual(len(self.lst_run), TodoUpgrade.MAX_ERROR_RETRY) + + def test_giving_up_says_so_out_loud(self): + # S'arrêter en silence ferait croire que c'est réparé. + obj = self.upgrade(resets=[True] * 20) + text = self.executer(obj) + self.assertIn("needs a developer", text) + + def test_the_loop_is_bounded_STRUCTURALLY(self): + """Même si la bascule du défaut disparaissait un jour. + + Deux protections pour la même panne, et c'est délibéré : celle-ci + ne dépend d'aucune logique métier. Une boucle infinie dans une + migration lancée sans surveillance coûte une nuit, et la seule + preuve qu'une boucle se termine est de compter ses tours. + """ + obj = self.upgrade(lst_answer=["2"] * 50, resets=[]) + self.executer(obj) + self.assertLessEqual(len(self.lst_run), 2) + + def test_the_turn_bound_says_so(self): + obj = self.upgrade(lst_answer=["2"] * 50, resets=[]) + text = self.executer(obj) + self.assertIn("Too many turns", text) + + def test_the_bound_is_three(self): + self.assertEqual(TodoUpgrade.MAX_ERROR_RETRY, 3) + + def test_a_command_that_succeeds_never_asks(self): + obj = self.upgrade(echec=False) + self.executer(obj) + self.assertEqual(self.lst_reset, []) + self.assertEqual(len(self.lst_run), 1) + + +class TestTheHumanKeepsTheWheel(Harness): + def test_typing_one_replays_without_consuming_the_budget(self): + # Le plafond borne le rejeu AUTOMATIQUE. Quelqu'un qui tape « 1 » + # sait ce qu'il fait, et se voir refuser un quatrième essai serait + # une surprise désagréable. + obj = self.upgrade(lst_answer=["1", "1", "1", "1", ""]) + self.executer(obj) + self.assertEqual(len(self.lst_run), 5) + + def test_typing_n_continues_without_repairing(self): + obj = self.upgrade(lst_answer=[""] * 0 + ["x"]) + self.executer(obj) + self.assertEqual(self.lst_reset, []) + self.assertEqual(len(self.lst_run), 1) + + def test_the_prompt_says_what_enter_does(self): + import inspect + + source = inspect.getsource(TodoUpgrade.todo_upgrade_execute) + self.assertIn('defaut = "3" if database_name else ""', source) + self.assertIn("default=defaut", source) + + def test_without_a_database_the_default_is_to_continue(self): + # Les options 2 à 4 ne s'affichent pas : proposer « 3 » viserait + # une commande qui n'existe pas. + obj = self.upgrade() + obj.database_from_command = lambda cmd: None + self.executer(obj) + self.assertEqual(self.lst_reset, []) + self.assertEqual(len(self.lst_run), 1) + + +class TestTheResetReportsWhatItDid(unittest.TestCase): + """Sans verdict, on ne peut pas décider de rejouer.""" + + def test_nothing_drifted_is_False(self): + obj = TodoUpgrade.__new__(TodoUpgrade) + obj.stale_cow_keys = lambda db: [] + with redirect_stdout(io.StringIO()): + self.assertFalse(obj.prompt_reset_stale_cow_views("db")) + + def test_saying_no_is_False(self): + obj = TodoUpgrade.__new__(TodoUpgrade) + obj.stale_cow_keys = lambda db: ["web.layout"] + obj.ask_gate = lambda prompt, default="": "n" + with redirect_stdout(io.StringIO()): + self.assertFalse(obj.prompt_reset_stale_cow_views("db")) + + def test_a_reset_that_ran_is_True(self): + obj = TodoUpgrade.__new__(TodoUpgrade) + obj.stale_cow_keys = lambda db: ["web.layout"] + obj.ask_gate = lambda prompt, default="": default + obj.run_on_terminal = lambda cmd: 0 + with redirect_stdout(io.StringIO()): + self.assertTrue(obj.prompt_reset_stale_cow_views("db")) + + def test_a_reset_that_FAILED_is_False(self): + # Rejouer derrière une réinitialisation qui a échoué, c'est brûler + # une tentative sur un état inchangé. + obj = TodoUpgrade.__new__(TodoUpgrade) + obj.stale_cow_keys = lambda db: ["web.layout"] + obj.ask_gate = lambda prompt, default="": default + obj.run_on_terminal = lambda cmd: 2 + with redirect_stdout(io.StringIO()): + self.assertFalse(obj.prompt_reset_stale_cow_views("db")) + + +if __name__ == "__main__": + unittest.main()