[FIX] addons: stop calling a leftover report a failed command
« Command returned error code: 1 » followed a theme that was removed correctly. 1 is this toolkit's « there are findings », but the report was the uninstaller's last command, so its code became the script's; and the COW check was still read through the capturing executor, which announces any non-zero code as an error. The leftovers can now be dealt with instead of only listed: keep by default, or delete after their content is written to private/. Listing fifteen attachments and stopping there meant composing an unlink() by hand, mid-migration, from identifiers read off a screen. --- FR --- [FIX] addons : cesser d'appeler « erreur » un rapport de restes « Command returned error code: 1 » suivait un thème correctement retiré. 1 veut dire « il y a des constats » dans cet outillage, mais le rapport était la dernière commande du désinstalleur, donc son code devenait celui du script ; et la vérification COW passait encore par l'exécuteur qui capture, lequel annonce tout code non nul comme une erreur. Les restes peuvent désormais être traités, pas seulement listés : garder par défaut, ou effacer après écriture de leur contenu sous private/. Lister quinze pièces jointes et s'arrêter là revenait à faire composer un unlink() à la main, en pleine migration. Assisted-by: Claude Opus 5 (cherry picked from commit 304ce32b2680e3d01e38c3224bb85c2798cd6f53)
This commit is contained in:
parent
7e74455d63
commit
ec6ac3a188
5 changed files with 289 additions and 6 deletions
|
|
@ -121,12 +121,118 @@ def render(theme, attachments, views):
|
|||
return "\n".join(lines) + "\n"
|
||||
|
||||
|
||||
def backup_attachments(database, theme, lst_row, filestore=None):
|
||||
"""Écrire le contenu des pièces jointes AVANT de les supprimer.
|
||||
|
||||
C'est la condition pour pouvoir répondre « efface ». Sans elle, on
|
||||
détruirait ce dont on vient d'écrire qu'il peut être la seule trace
|
||||
d'une personnalisation.
|
||||
"""
|
||||
base = filestore or os.path.join(
|
||||
os.path.expanduser("~"),
|
||||
".local",
|
||||
"share",
|
||||
"Odoo",
|
||||
"filestore",
|
||||
database,
|
||||
)
|
||||
directory = os.path.join(
|
||||
"private", "odoo", "migration", database, "theme_backup", theme
|
||||
)
|
||||
os.makedirs(directory, exist_ok=True)
|
||||
lst_saved = []
|
||||
for row in lst_row:
|
||||
att_id = row.split("|")[0]
|
||||
rows = run_psql(
|
||||
database,
|
||||
"SELECT COALESCE(store_fname, '') FROM ir_attachment"
|
||||
f" WHERE id = {int(att_id)};",
|
||||
)
|
||||
store_fname = rows[0].strip() if rows else ""
|
||||
if not store_fname:
|
||||
continue
|
||||
source = os.path.join(base, store_fname)
|
||||
if not os.path.isfile(source):
|
||||
continue
|
||||
target = os.path.join(
|
||||
directory, f"{att_id}_" + os.path.basename(row.split("|")[1])
|
||||
)
|
||||
with open(source, "rb") as src, open(target, "wb") as dst:
|
||||
dst.write(src.read())
|
||||
lst_saved.append(target)
|
||||
return lst_saved
|
||||
|
||||
|
||||
def delete_attachments(database, lst_row, config_path="./config.conf"):
|
||||
"""Supprimer par le shell d'Odoo, pour qu'il gère aussi le filestore.
|
||||
|
||||
Un DELETE en SQL laisserait les fichiers orphelins et les caches
|
||||
incohérents ; unlink() fait le ménage complet, dans toutes les versions.
|
||||
"""
|
||||
lst_id = [row.split("|")[0] for row in lst_row]
|
||||
script = (
|
||||
f"env['ir.attachment'].browse({lst_id!r}).unlink()\n"
|
||||
"env.cr.commit()\n"
|
||||
)
|
||||
done = subprocess.run(
|
||||
["./odoo_bin.sh", "shell", "-c", config_path, "-d", database],
|
||||
input=script,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
return done.returncode, done.stdout + done.stderr
|
||||
|
||||
|
||||
def prompt(database, theme, attachments, views, config_path, ask=input):
|
||||
"""Garder ou effacer. « Garder » par défaut, et la sauvegarde d'abord."""
|
||||
if not attachments:
|
||||
return False
|
||||
answer = (
|
||||
ask(
|
||||
f"💬 {t('Delete these leftovers, or keep them?')}"
|
||||
f" ({t('Enter = keep')}, d = {t('delete, after saving them')}) : "
|
||||
)
|
||||
.strip()
|
||||
.lower()
|
||||
)
|
||||
if answer != "d":
|
||||
print(f"ℹ -> {t('Kept. Nothing was deleted.')}")
|
||||
return False
|
||||
lst_saved = backup_attachments(database, theme, attachments)
|
||||
print(f"📦 {t('Saved before deleting')} : {len(lst_saved)}")
|
||||
if lst_saved:
|
||||
print(f" {os.path.dirname(lst_saved[0])}")
|
||||
status, output = delete_attachments(database, attachments, config_path)
|
||||
print(output.strip()[-1500:])
|
||||
if status:
|
||||
print(f"❌ {t('Deletion failed, nothing was removed.')}")
|
||||
return False
|
||||
print(f"✅ -> {len(attachments)} {t('attachment(s) deleted.')}")
|
||||
return True
|
||||
|
||||
|
||||
def main(argv=None):
|
||||
parser = argparse.ArgumentParser(
|
||||
description=("List what an uninstalled theme left behind (read-only).")
|
||||
)
|
||||
parser.add_argument("-d", "--database", required=True)
|
||||
parser.add_argument("-t", "--theme", required=True)
|
||||
parser.add_argument(
|
||||
"-c",
|
||||
"--config",
|
||||
default="./config.conf",
|
||||
help="Odoo config used by the shell for --delete",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--delete",
|
||||
action="store_true",
|
||||
help="delete them (WRITES; saves their content first)",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--report-only",
|
||||
action="store_true",
|
||||
help="never ask anything, even in front of a terminal",
|
||||
)
|
||||
config = parser.parse_args(argv)
|
||||
try:
|
||||
attachments, views = collect(config.database, config.theme)
|
||||
|
|
@ -134,7 +240,32 @@ def main(argv=None):
|
|||
print(f"❌ {exc}")
|
||||
return 2
|
||||
print(render(config.theme, attachments, views))
|
||||
return 1 if (attachments or views) else 0
|
||||
if not attachments and not views:
|
||||
return 0
|
||||
if config.delete:
|
||||
lst_saved = backup_attachments(
|
||||
config.database, config.theme, attachments
|
||||
)
|
||||
print(f"📦 {t('Saved before deleting')} : {len(lst_saved)}")
|
||||
status, output = delete_attachments(
|
||||
config.database, attachments, config.config
|
||||
)
|
||||
print(output.strip()[-1500:])
|
||||
if status:
|
||||
print(f"❌ {t('Deletion failed, nothing was removed.')}")
|
||||
return 2
|
||||
print(f"✅ -> {len(attachments)} {t('attachment(s) deleted.')}")
|
||||
return 0
|
||||
if not config.report_only and sys.stdin.isatty():
|
||||
if prompt(
|
||||
config.database,
|
||||
config.theme,
|
||||
attachments,
|
||||
views,
|
||||
config.config,
|
||||
):
|
||||
return 0
|
||||
return 1
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
|
|
|||
|
|
@ -89,4 +89,8 @@ fi
|
|||
# sont parties, mais elles survivent à toutes les migrations suivantes et
|
||||
# personne ne sait plus d'où elles viennent. On les signale, on ne les
|
||||
# supprime pas : leur contenu peut être la seule trace d'une personnalisation.
|
||||
./script/addons/theme_leftover.py -d "$DATABASE" -t "$THEME"
|
||||
# Son code de sortie dit « il reste des choses », pas « la désinstallation a
|
||||
# échoué » : le laisser devenir celui du script faisait annoncer une erreur
|
||||
# sur un thème correctement retiré.
|
||||
./script/addons/theme_leftover.py -d "$DATABASE" -t "$THEME" -c "$CONFIG" || true
|
||||
exit 0
|
||||
|
|
|
|||
|
|
@ -4950,6 +4950,34 @@ TRANSLATIONS = {
|
|||
"fr": "Choix inconnu, rien n'a été réinitialisé.",
|
||||
"en": "Unknown choice, nothing was reset.",
|
||||
},
|
||||
"Delete these leftovers, or keep them?": {
|
||||
"fr": "Effacer ces restes, ou les garder ?",
|
||||
"en": "Delete these leftovers, or keep them?",
|
||||
},
|
||||
"Enter = keep": {
|
||||
"fr": "Entrée = garder",
|
||||
"en": "Enter = keep",
|
||||
},
|
||||
"delete, after saving them": {
|
||||
"fr": "effacer, après les avoir sauvegardés",
|
||||
"en": "delete, after saving them",
|
||||
},
|
||||
"Kept. Nothing was deleted.": {
|
||||
"fr": "Conservés. Rien n'a été effacé.",
|
||||
"en": "Kept. Nothing was deleted.",
|
||||
},
|
||||
"Saved before deleting": {
|
||||
"fr": "Sauvegardé avant effacement",
|
||||
"en": "Saved before deleting",
|
||||
},
|
||||
"Deletion failed, nothing was removed.": {
|
||||
"fr": "L'effacement a échoué, rien n'a été retiré.",
|
||||
"en": "Deletion failed, nothing was removed.",
|
||||
},
|
||||
"attachment(s) deleted.": {
|
||||
"fr": "pièce(s) jointe(s) effacée(s).",
|
||||
"en": "attachment(s) deleted.",
|
||||
},
|
||||
"Nothing to decide yet": {
|
||||
"fr": "Rien à décider pour l'instant",
|
||||
"en": "Nothing to decide yet",
|
||||
|
|
|
|||
|
|
@ -1624,11 +1624,13 @@ class TodoUpgrade:
|
|||
# -- before hours of migration -- leaves time to arbitrate.
|
||||
# Only the next bump can be predicted: the modes in database describe
|
||||
# the current version.
|
||||
status, cmd_executed, output = self.todo_upgrade_execute(
|
||||
# Sur le terminal, sans capturer : plus rien ne relit cette sortie —
|
||||
# la décision se prend sur le code de retour — et la capture faisait
|
||||
# annoncer « Command returned error code: 1 » sur un rapport qui va
|
||||
# bien. 1 veut dire « des copies casseront », pas « l'outil a raté ».
|
||||
status = self.run_on_terminal(
|
||||
f"{PYTHON_BIN} ./script/odoo/migration/check_cow_views.py"
|
||||
f" -d {database_name} -t odoo{start_version + 1}.0",
|
||||
get_output=True,
|
||||
wait_at_error=False,
|
||||
f" -d {database_name} -t odoo{start_version + 1}.0"
|
||||
)
|
||||
# L'avertissement annonçait un problème, invitait à arbitrer, et
|
||||
# aucune question ne suivait : dire « la question viendra au palier »
|
||||
|
|
|
|||
|
|
@ -213,6 +213,124 @@ class TestTheMigrationOffersIt(unittest.TestCase):
|
|||
self.assertIn("name <> 'theme_default'", source)
|
||||
|
||||
|
||||
class TestKeepOrDeleteTheLeftovers(unittest.TestCase):
|
||||
"""Signaler sans offrir le geste oblige à le composer soi-même.
|
||||
|
||||
Le rapport listait quinze pièces jointes et s'arrêtait là. Les effacer
|
||||
demandait de retrouver les identifiants et d'écrire un unlink() à la
|
||||
main — au milieu d'une migration, c'est ce qu'on ne fait pas.
|
||||
|
||||
« Garder » reste le défaut, et rien n'est effacé sans avoir été écrit sur
|
||||
disque d'abord : c'est la condition pour pouvoir répondre « efface ».
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
from script.todo import todo_i18n
|
||||
|
||||
self.addCleanup(
|
||||
setattr, todo_i18n, "_current_lang", todo_i18n._current_lang
|
||||
)
|
||||
todo_i18n._current_lang = "en"
|
||||
self.rows = ["4457|/theme_x/static/a.scss|2021-03-04"]
|
||||
self.saved = []
|
||||
self.deleted = []
|
||||
self.original_backup = theme_leftover.backup_attachments
|
||||
self.original_delete = theme_leftover.delete_attachments
|
||||
theme_leftover.backup_attachments = (
|
||||
lambda db, th, rows, fs=None: self.saved.append(rows) or ["/tmp/x"]
|
||||
)
|
||||
theme_leftover.delete_attachments = (
|
||||
lambda db, rows, cfg="./config.conf": (
|
||||
self.deleted.append(rows),
|
||||
(0, "ok"),
|
||||
)[1]
|
||||
)
|
||||
self.addCleanup(
|
||||
setattr,
|
||||
theme_leftover,
|
||||
"backup_attachments",
|
||||
self.original_backup,
|
||||
)
|
||||
self.addCleanup(
|
||||
setattr, theme_leftover, "delete_attachments", self.original_delete
|
||||
)
|
||||
|
||||
def run_prompt(self, answer, rows=None):
|
||||
import contextlib
|
||||
import io
|
||||
|
||||
out = io.StringIO()
|
||||
with contextlib.redirect_stdout(out):
|
||||
done = theme_leftover.prompt(
|
||||
"db",
|
||||
"theme_x",
|
||||
self.rows if rows is None else rows,
|
||||
[],
|
||||
"./config.conf",
|
||||
ask=lambda prompt: answer,
|
||||
)
|
||||
return done, out.getvalue()
|
||||
|
||||
def test_enter_keeps_them(self):
|
||||
done, text = self.run_prompt("")
|
||||
self.assertFalse(done)
|
||||
self.assertEqual(self.deleted, [])
|
||||
self.assertIn("Kept", text)
|
||||
|
||||
def test_d_saves_before_deleting(self):
|
||||
# L'ORDRE est le point : effacer d'abord rendrait la sauvegarde vide.
|
||||
done, _ = self.run_prompt("d")
|
||||
self.assertTrue(done)
|
||||
self.assertEqual(len(self.saved), 1)
|
||||
self.assertEqual(len(self.deleted), 1)
|
||||
|
||||
def test_a_failed_deletion_says_nothing_was_removed(self):
|
||||
theme_leftover.delete_attachments = (
|
||||
lambda db, rows, cfg="./config.conf": (1, "boom")
|
||||
)
|
||||
done, text = self.run_prompt("d")
|
||||
self.assertFalse(done)
|
||||
self.assertIn("nothing was removed", text)
|
||||
|
||||
def test_no_leftover_asks_nothing(self):
|
||||
done, text = self.run_prompt("d", rows=[])
|
||||
self.assertFalse(done)
|
||||
self.assertEqual(text, "")
|
||||
|
||||
def test_the_prompt_stays_out_of_a_pipe(self):
|
||||
with open(theme_leftover.__file__) as handle:
|
||||
self.assertIn("sys.stdin.isatty()", handle.read())
|
||||
|
||||
|
||||
class TestTheMisleadingErrorCode(unittest.TestCase):
|
||||
"""« 1 » veut dire « il reste des choses », pas « ça a raté »."""
|
||||
|
||||
def test_the_uninstaller_does_not_fail_on_leftovers(self):
|
||||
# Le rapport était la dernière commande du script : son code
|
||||
# devenait celui du script, et la migration annonçait une erreur sur
|
||||
# un thème correctement retiré.
|
||||
with open(SCRIPT) as handle:
|
||||
source = handle.read()
|
||||
self.assertIn("theme_leftover.py", source)
|
||||
queue = source[source.index("theme_leftover.py") :]
|
||||
self.assertIn("|| true", queue)
|
||||
self.assertIn("exit 0", queue)
|
||||
|
||||
def test_the_cow_check_no_longer_goes_through_the_capturing_executor(self):
|
||||
# exec_command_live imprime « Command returned error code: 1 » dès
|
||||
# qu'un code non nul sort, y compris sur un rapport qui va bien.
|
||||
import inspect
|
||||
|
||||
from script.todo.todo_upgrade import TodoUpgrade
|
||||
|
||||
source = inspect.getsource(TodoUpgrade.execute_odoo_upgrade)
|
||||
avant = source[: source.index("check_cow_views.py")]
|
||||
self.assertGreater(
|
||||
avant.rfind("run_on_terminal("),
|
||||
avant.rfind("todo_upgrade_execute("),
|
||||
)
|
||||
|
||||
|
||||
class TestExitCodes(unittest.TestCase):
|
||||
"""0 rien, 1 des restes, 2 l'outil a échoué — comme les outils voisins."""
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue