From 1b2ad61771c68a56acbb9354c23e5805d2ed5933 Mon Sep 17 00:00:00 2001 From: Connor Sheehan Date: Thu, 23 Oct 2025 13:56:50 -0400 Subject: [PATCH 1/3] landing_jobs: store commit SHA for each revision/landing job combo (Bug 1995895) Store the `commit_id` of the newly created commit on the `Revision` object during patch application. On successful push, set the current `commit_id` of the revision on the intermediate table before transitioning job status to `LANDED`. --- .../api/legacy/workers/landing_worker.py | 6 ++ src/lando/api/tests/test_landings.py | 99 +++++++++++++++++++ .../0032_revisionlandingjob_commit_id.py | 18 ++++ src/lando/main/models/landing_job.py | 8 ++ src/lando/main/models/revision.py | 3 + 5 files changed, 134 insertions(+) create mode 100644 src/lando/main/migrations/0032_revisionlandingjob_commit_id.py diff --git a/src/lando/api/legacy/workers/landing_worker.py b/src/lando/api/legacy/workers/landing_worker.py index 969e02b2c..26b3abae3 100644 --- a/src/lando/api/legacy/workers/landing_worker.py +++ b/src/lando/api/legacy/workers/landing_worker.py @@ -141,6 +141,7 @@ def run_job(self, job: LandingJob) -> bool: except TemporaryFailureException: return False + job.set_landed_commit_ids() job.transition_status(JobAction.LAND, commit_id=commit_id) mots_path = Path(repo.path) / "mots.yaml" @@ -241,6 +242,11 @@ def apply_and_push( raise PermanentFailureException(message) from exc else: new_commit = scm.describe_commit() + + # Record the commit ID on the revision object. + revision.commit_id = new_commit.hash + revision.save() + logger.debug(f"Created new commit {new_commit}") # Get the changeset titles for the stack. diff --git a/src/lando/api/tests/test_landings.py b/src/lando/api/tests/test_landings.py index 47ea829a4..0d8c5f10c 100644 --- a/src/lando/api/tests/test_landings.py +++ b/src/lando/api/tests/test_landings.py @@ -16,6 +16,7 @@ LandingJob, Repo, Revision, + RevisionLandingJob, ) from lando.main.scm import SCM_TYPE_GIT, SCM_TYPE_HG from lando.main.scm.exceptions import SCMInternalServerError @@ -381,6 +382,104 @@ def test_integrated_execute_job( assert new_push_count == 1, "Incorrect number of additional pushes in the PushLog" +@pytest.mark.parametrize( + "repo_type", + [ + SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) +@pytest.mark.django_db +def test_revisionlandingjob_commit_ids_updated_on_success( + repo_mc, + treestatusdouble, + mock_phab_trigger_repo_update_apply_async, + create_patch_revision, + make_landing_job, + get_landing_worker, + repo_type: str, +): + """Ensure landed commit SHAs are copied onto RevisionLandingJob rows.""" + repo = repo_mc(repo_type) + treestatusdouble.open_tree(repo.name) + + revisions = [ + create_patch_revision(1, patch=None), + create_patch_revision(2, patch=None), + ] + job_params = { + "status": JobStatus.IN_PROGRESS, + "requester_email": "test@example.com", + "target_repo": repo, + "attempts": 1, + } + job = make_landing_job(revisions=revisions, **job_params) + + worker = get_landing_worker(repo_type) + assert worker.run_job(job) + assert job.status == JobStatus.LANDED + + revision_jobs = list( + RevisionLandingJob.objects.filter(landing_job=job).order_by("index") + ) + assert len(revision_jobs) == len(revisions) + + ordered_revisions = list(job.revisions) + for revision, revision_job in zip(ordered_revisions, revision_jobs, strict=False): + assert revision.commit_id, "`commit_id` should be set on `Revision` object." + assert ( + revision_job.commit_id + ), "`commit_id` should be set on `RevisionLandingJob` object." + + +@pytest.mark.parametrize( + "repo_type", + [ + SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) +@pytest.mark.django_db +def test_revisionlandingjob_commit_ids_unset_without_landing( + repo_mc, + treestatusdouble, + mock_phab_trigger_repo_update_apply_async, + create_patch_revision, + make_landing_job, + get_landing_worker, + repo_type: str, +): + """Ensure `commit_id` is not tracked for incomplete job.""" + repo = repo_mc(repo_type) + treestatusdouble.open_tree(repo.name) + scm = repo.scm + + job_params = { + "status": JobStatus.IN_PROGRESS, + "requester_email": "test@example.com", + "target_repo": repo, + "attempts": 1, + } + job = make_landing_job(revisions=[create_patch_revision(1)], **job_params) + + scm.push = mock.MagicMock(side_effect=SCMInternalServerError("push failed", "500")) + + worker = get_landing_worker(repo_type) + assert not worker.run_job(job) + assert job.status == JobStatus.DEFERRED + + revision_jobs = list( + RevisionLandingJob.objects.filter(landing_job=job).order_by("index") + ) + assert len(revision_jobs) == 1 + + revision = job.revisions.first() + assert revision.commit_id, "`commit_id` should still be set on `Revision` object." + assert ( + revision_jobs[0].commit_id is None + ), "`commit_id` should not be set for un-landed job." + + @pytest.mark.parametrize( "repo_type", [ diff --git a/src/lando/main/migrations/0032_revisionlandingjob_commit_id.py b/src/lando/main/migrations/0032_revisionlandingjob_commit_id.py new file mode 100644 index 000000000..f00613789 --- /dev/null +++ b/src/lando/main/migrations/0032_revisionlandingjob_commit_id.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.7 on 2025-10-23 17:57 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("main", "0031_alter_landingjob_requester_email"), + ] + + operations = [ + migrations.AddField( + model_name="revisionlandingjob", + name="commit_id", + field=models.CharField(blank=True, max_length=40, null=True), + ), + ] diff --git a/src/lando/main/models/landing_job.py b/src/lando/main/models/landing_job.py index db5278861..52c273a47 100644 --- a/src/lando/main/models/landing_job.py +++ b/src/lando/main/models/landing_job.py @@ -220,6 +220,14 @@ def set_landed_revision_diffs(self): revision=revision, landing_job=self ).update(diff_id=revision.diff_id) + def set_landed_commit_ids(self): + """Assign diff_ids, if available, to each association row.""" + # Update association table records with current diff_id values. + for revision in self.unsorted_revisions.all(): + RevisionLandingJob.objects.filter( + revision=revision, landing_job=self + ).update(commit_id=revision.commit_id) + def set_landed_reviewers(self, path: Path): """Set approving peers and owners at time of landing.""" directory = Directory(FileConfig(path)) diff --git a/src/lando/main/models/revision.py b/src/lando/main/models/revision.py index 3037da0a1..d24af1dd0 100644 --- a/src/lando/main/models/revision.py +++ b/src/lando/main/models/revision.py @@ -31,6 +31,9 @@ class RevisionLandingJob(BaseModel): # See also: LandingJob.set_landed_revision_diffs, called by transplants.post. diff_id = models.IntegerField(null=True, blank=True) + # The commit ID generated by the landing worker, before pushing to remote repo. + commit_id = models.CharField(max_length=40, null=True, blank=True) + class Revision(BaseModel): """ From eafa9bc37b82140c3a62deee0c807b151084e260 Mon Sep 17 00:00:00 2001 From: Connor Sheehan Date: Thu, 23 Oct 2025 15:35:29 -0400 Subject: [PATCH 2/3] fix docstring --- src/lando/main/models/landing_job.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lando/main/models/landing_job.py b/src/lando/main/models/landing_job.py index 52c273a47..281de558f 100644 --- a/src/lando/main/models/landing_job.py +++ b/src/lando/main/models/landing_job.py @@ -221,7 +221,7 @@ def set_landed_revision_diffs(self): ).update(diff_id=revision.diff_id) def set_landed_commit_ids(self): - """Assign diff_ids, if available, to each association row.""" + """Assign `commit_id`, if available, to each association row.""" # Update association table records with current diff_id values. for revision in self.unsorted_revisions.all(): RevisionLandingJob.objects.filter( From a5b39f49963bc19478c0bd1c010f1bf62961f701 Mon Sep 17 00:00:00 2001 From: Connor Sheehan Date: Thu, 23 Oct 2025 15:36:05 -0400 Subject: [PATCH 3/3] remove redundant comment --- src/lando/main/models/landing_job.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lando/main/models/landing_job.py b/src/lando/main/models/landing_job.py index 281de558f..4b28b4428 100644 --- a/src/lando/main/models/landing_job.py +++ b/src/lando/main/models/landing_job.py @@ -222,7 +222,6 @@ def set_landed_revision_diffs(self): def set_landed_commit_ids(self): """Assign `commit_id`, if available, to each association row.""" - # Update association table records with current diff_id values. for revision in self.unsorted_revisions.all(): RevisionLandingJob.objects.filter( revision=revision, landing_job=self