From cfb4598878fec075bdd18866f5736bfe2608f4e2 Mon Sep 17 00:00:00 2001 From: Zeid Zabaneh Date: Fri, 9 Feb 2024 10:48:24 -0500 Subject: [PATCH] landing_worker: delete patch content after landing (bug 1879536) - delete patch content after landing - add one-time cli command to do this for previously landed jobs - update tests to reflect new behaviour --- landoapi/cli.py | 14 ++++++ landoapi/workers/landing_worker.py | 5 +++ tests/test_try.py | 71 ++++++++++++++++-------------- 3 files changed, 58 insertions(+), 32 deletions(-) diff --git a/landoapi/cli.py b/landoapi/cli.py index 5065b3a5..525f7c38 100644 --- a/landoapi/cli.py +++ b/landoapi/cli.py @@ -107,6 +107,20 @@ def run_post_deploy_sequence(): ) +@cli.command(name="clean-landing-job-patches") +def clean_landing_job_patches(): + """Iterate over all landed jobs and delete their patches.""" + from landoapi.models.landing_job import LandingJob, LandingJobStatus + from landoapi.storage import db, db_subsystem + + db_subsystem.ensure_ready() + jobs = LandingJob.query.filter(LandingJob.status == LandingJobStatus.LANDED).all() + for job in jobs: + for revision in job.revisions: + revision.patch_bytes = b"" + db.session.commit() + + @cli.command(context_settings={"ignore_unknown_options": True}) @click.argument("celery_arguments", nargs=-1, type=click.UNPROCESSED) def celery(celery_arguments): diff --git a/landoapi/workers/landing_worker.py b/landoapi/workers/landing_worker.py index 67ef99e4..4e69265a 100644 --- a/landoapi/workers/landing_worker.py +++ b/landoapi/workers/landing_worker.py @@ -439,6 +439,11 @@ def run_job( LandingJobAction.LAND, commit_id=commit_id, commit=True, db=db ) + # Patches are no longer necessary, delete them. + for revision in job.revisions: + revision.patch_bytes = b"" + db.session.commit() + mots_path = Path(hgrepo.path) / "mots.yaml" if mots_path.exists(): logger.info(f"{mots_path} found, setting reviewer data.") diff --git a/tests/test_try.py b/tests/test_try.py index 8a6e42af..8614ffa5 100644 --- a/tests/test_try.py +++ b/tests/test_try.py @@ -245,6 +245,23 @@ def test_try_api_success_hgexport( worker = LandingWorker(sleep_seconds=0.01) hgrepo = HgRepo(hg_clone.strpath) + assert len(job.revisions) == 1, "Job should be landing a single revision." + revision = job.revisions[0] + assert revision.patch_bytes == ( + b"# HG changeset patch\n" + b"# User Test User \n" + b"# Date 0 +0000\n" + b"# Diff Start Line 6\n" + b"add another file.\n" + b"\n" + b"diff --git a/test.txt b/test.txt\n" + b"--- a/test.txt\n" + b"+++ b/test.txt\n" + b"@@ -1,1 +1,2 @@\n" + b" TEST\n" + b"+adding another line" + ), "Patch diff should be parsed from patch body." + assert worker.run_job(job, repo, hgrepo, treestatus) assert job.status == LandingJobStatus.LANDED assert len(job.landed_commit_id) == 40 @@ -267,20 +284,7 @@ def test_try_api_success_hgexport( assert ( revision.patch_data["timestamp"] == "0" ), "Timestamp should be parsed from `Date` header." - assert revision.patch_bytes == ( - b"# HG changeset patch\n" - b"# User Test User \n" - b"# Date 0 +0000\n" - b"# Diff Start Line 6\n" - b"add another file.\n" - b"\n" - b"diff --git a/test.txt b/test.txt\n" - b"--- a/test.txt\n" - b"+++ b/test.txt\n" - b"@@ -1,1 +1,2 @@\n" - b" TEST\n" - b"+adding another line" - ), "Patch diff should be parsed from patch body." + assert revision.patch_bytes == b"", "Patch diff should be cleared after landing." def test_try_api_success_gitformatpatch( @@ -334,6 +338,26 @@ def test_try_api_success_gitformatpatch( worker = LandingWorker(sleep_seconds=0.01) hgrepo = HgRepo(hg_clone.strpath) + assert len(job.revisions) == 1, "Job should be landing a single revision." + revision = job.revisions[0] + + assert revision.patch_bytes == ( + b"# HG changeset patch\n" + b"# User Connor Sheehan \n" + b"# Date 1657139769 +0000\n" + b"# Diff Start Line 8\n" + b"add another file\n" + b"\n" + b"add another file to the repo.\n" + b"\n" + b"diff --git a/test.txt b/test.txt\n" + b"--- a/test.txt\n" + b"+++ b/test.txt\n" + b"@@ -1,1 +1,2 @@\n" + b" TEST\n" + b"+adding another line" + ), "Patch diff should be parsed from patch body." + # Assert the job landed against the expected commit hash. assert worker.run_job(job, repo, hgrepo, treestatus) assert job.status == LandingJobStatus.LANDED @@ -343,8 +367,6 @@ def test_try_api_success_gitformatpatch( ), "Target changeset should match the passed value." # Test the revision content matches expected. - assert len(job.revisions) == 1, "Job should have landed a single revision." - revision = job.revisions[0] assert ( revision.patch_data["author_name"] == "Connor Sheehan" ), "Patch author should be parsed from `From` header." @@ -357,19 +379,4 @@ def test_try_api_success_gitformatpatch( assert ( revision.patch_data["timestamp"] == "1657139769" ), "Timestamp should be parsed from `Date` header." - assert revision.patch_bytes == ( - b"# HG changeset patch\n" - b"# User Connor Sheehan \n" - b"# Date 1657139769 +0000\n" - b"# Diff Start Line 8\n" - b"add another file\n" - b"\n" - b"add another file to the repo.\n" - b"\n" - b"diff --git a/test.txt b/test.txt\n" - b"--- a/test.txt\n" - b"+++ b/test.txt\n" - b"@@ -1,1 +1,2 @@\n" - b" TEST\n" - b"+adding another line" - ), "Patch diff should be parsed from patch body." + assert revision.patch_bytes == b"", "Patch diff should be cleared after landing."