Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions src/lando/api/legacy/workers/landing_worker.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import configparser
import json
import logging
import subprocess
from pathlib import Path
Expand Down Expand Up @@ -172,15 +173,27 @@ def run_job(self, job: LandingJob) -> bool:

return True

def add_try_task_config(self, scm: AbstractSCM):
with (Path(scm.path) / "try_task_config.json").open("x") as f:
data = {
"parameters": {
"optimize_target_tasks": True,
"target_tasks_method": "codereview",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this firefox patch is approved, you'll be able to pass all the kwargs in try_task_config parameter.

Your payload should look like:

data {
    "parameters": {
        "optimize_target_tasks": true,
        "target_tasks_method": "codereview",
        "try_mode": "try_task_config",
        "try_task_config": { f"github_{k}": v for k, v in kwargs.items()}
        }
    },
    "version": 2
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I am going to move these changes to their own PR so that this one can be merged before the schema is finalized. Will update with your suggestion in that other PR.

},
"version": 2,
}
content = json.dumps(data)
f.write(content)

def convert_patches_to_diff(self, scm: AbstractSCM, job: LandingJob):
"""Generate a unified diff from multiple patches stored in a revision."""
# NOTE: this only applies to git patches that are downloaded from GitHub
# at this time. In theory this would work for any provided patches in a
# standard format.

def get_diff_from_patches(revision: Revision) -> str:
def add_diff_from_patches(revision: Revision) -> str:
logger.debug(f"Converting paches to single diff for {revision} ...")
return scm.get_diff_from_patches(revision.patches)
return scm.add_diff_from_patches(revision.patches)

# NOTE: this is only supported for jobs with a single revision at this time.
# See bug 2001185.
Expand All @@ -197,7 +210,7 @@ def get_diff_from_patches(revision: Revision) -> str:
raise ValueError("Revision is missing patches.")

diff = self.handle_new_commit_failures(
get_diff_from_patches,
add_diff_from_patches,
job.target_repo,
job,
scm,
Expand Down Expand Up @@ -230,6 +243,19 @@ def apply_patch(revision: Revision):

self.update_repo(repo, job, scm, job.target_commit_hash)

if job.is_pull_request_job and job.handover_repo:
if not job.handover_repo.is_try:
raise ValueError(
f"{job} handover to non-try repo ({job.handover_repos}) is not supported"
)

self.add_try_task_config(scm)
self.convert_patches_to_diff(scm, job)
job.handover()
message = "Job deferred to try repo."
job.transition_status(JobAction.DEFER, message=message)
raise TemporaryFailureException(message)

if job.is_pull_request_job:
self.convert_patches_to_diff(scm, job)
self.update_repo(repo, job, scm, job.target_commit_hash)
Expand Down
45 changes: 45 additions & 0 deletions src/lando/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,3 +305,48 @@ def get(self, request: WSGIRequest, repo_name: str, number: int) -> JsonResponse
target_repo, pull_request, request
)
return JsonResponse(warnings_and_blockers)


class PullRequestTryPushAPIView(APIView):
@method_decorator(require_authenticated_user)
def post(
self, request: WSGIRequest, repo_name: str, pull_number: int
) -> JsonResponse:
try:
try_repo = Repo.objects.get(name="try")
except Repo.DoesNotExist:
return JsonResponse({"errors": ["Try repo does not exist"]}, 500)

target_repo = Repo.objects.get(name=repo_name)
client = GitHubAPIClient(target_repo.url)
ldap_username = request.user.email
pull_request = client.build_pull_request(pull_number)

job = LandingJob.objects.create(
target_repo=target_repo,
is_handed_over=False,
is_pull_request_job=True,
handover_repo=try_repo,
requester_email=ldap_username,
)
author_name, author_email = pull_request.author
try:
timestamp = int(datetime.fromisoformat(pull_request.updated_at).timestamp())
except ValueError:
timestamp = int(datetime.now().timestamp())
patch_data = {
"author_name": author_name,
"author_email": author_email,
"commit_message": pull_request.commit_message,
"timestamp": timestamp,
}
Comment on lines +337 to +342
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for later: We've been building this often enough, we should make it a method of the PullRequest.

revision = Revision.objects.create(
pull_number=pull_request.number,
patches=pull_request.patch,
patch_data=patch_data,
)
add_revisions_to_job([revision], job)
job.status = JobStatus.SUBMITTED
job.save()

return JsonResponse({"id": job.id}, status=201)
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Generated by Django 5.2.8 on 2025-12-16 17:25

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("main", "0041_revision_pull_base_sha_revision_pull_head_sha"),
]

operations = [
migrations.AddField(
model_name="landingjob",
name="handover_repo",
field=models.ForeignKey(
blank=True,
null=True,
on_delete=django.db.models.deletion.SET_NULL,
related_name="handover_jobs",
to="main.repo",
),
),
migrations.AddField(
model_name="landingjob",
name="is_handed_over",
field=models.BooleanField(blank=True, null=True),
),
]
31 changes: 31 additions & 0 deletions src/lando/main/models/landing_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ class LandingJob(BaseJob):

is_pull_request_job = models.BooleanField(default=False, blank=True)

# Reference to the handover repo. A handover repo is a repo that a landing
# job is handed over to after being processed in a previous state. If this value
# is set, the job will be processed twice, but pushed once.
handover_repo = models.ForeignKey(
Repo,
on_delete=models.SET_NULL,
null=True,
blank=True,
related_name="handover_jobs",
)
is_handed_over = models.BooleanField(null=True, blank=True)

@property
def landed_phabricator_revisions(self) -> dict:
"""Return a mapping associating Phabricator revision IDs with the ID of the landed Diff."""
Expand Down Expand Up @@ -113,6 +125,25 @@ def serialized_landing_path(self) -> list[dict]:
for revision_id, diff_id in self.landed_revisions.items()
]

def handover(self):
if not self.is_pull_request_job:
raise NotImplementedError(
"Handover is only supported for pull request jobs"
)
if self.is_handed_over:
raise ValueError(f"{self} is already handed over")
if not self.handover_repo:
raise ValueError(f"{self} does not have a handover repo defined")
if not self.handover_repo.is_try:
raise NotImplementedError(
"Handover is currently only supported for try repo"
)

self.target_repo = self.handover_repo
self.is_pull_request_job = False
self.is_handed_over = True
self.save()

@property
def has_phabricator_revisions(self) -> bool:
"""Indicate if this job has Phabricator revisions by checking the first in the stack."""
Expand Down
6 changes: 6 additions & 0 deletions src/lando/main/models/repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
),
)

TRY_REPO_NAMES = ("try",)


def validate_path_in_repo_root(value: str):
path = Path(value)
Expand Down Expand Up @@ -101,6 +103,10 @@ class HooksChoices(models.TextChoices):
def path(self) -> str:
return str(self.system_path) or self.get_system_path()

@property
def is_try(self) -> bool:
return self.name in TRY_REPO_NAMES

# TODO: help text for fields below.
name = models.CharField(max_length=255, unique=True)
default_branch = models.CharField(max_length=255, default="", blank=True)
Expand Down
2 changes: 1 addition & 1 deletion src/lando/main/scm/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def apply_patch_git(self, patch_bytes: bytes):
raise exc

@detect_patch_conflict
def get_diff_from_patches(self, patches: str) -> str:
def add_diff_from_patches(self, patches: str) -> str:
Copy link
Member

@shtrom shtrom Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What motivates the change in name? We do a git add in the working directory, but ultimately this is transient, as it's only for the purpose of returning a diff string to the caller.

"""Apply multiple patches and return the diff output."""
# TODO: add error handling so that if something goes wrong here,
# a meaningful error is stored in the landing job. This would be
Expand Down
4 changes: 2 additions & 2 deletions src/lando/main/tests/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@ def test_GitSCM_process_merge_conflict_no_reject(
), "Missing default message from `content` in rejects_paths for that-other-file.txt"


def test_GitSCM_get_diff_from_patches(
def test_GitSCM_add_diff_from_patches(
git_patch: Callable,
git_repo: Path,
git_setup_user: Callable,
Expand Down Expand Up @@ -1128,5 +1128,5 @@ def test_GitSCM_get_diff_from_patches(
"""
).lstrip()

diff = scm.get_diff_from_patches(patches)
diff = scm.add_diff_from_patches(patches)
assert diff == expected_diff, "Did not generate expected diff from patches"
6 changes: 6 additions & 0 deletions src/lando/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
LandingJobPullRequestAPIView,
LegacyDiffWarningView,
PullRequestChecksAPIView,
PullRequestTryPushAPIView,
git2hgCommitMapView,
hg2gitCommitMapView,
)
Expand Down Expand Up @@ -116,6 +117,11 @@
LandingJobPullRequestAPIView.as_view(),
name="api-landing-job-pull-request",
),
path(
"api/pulls/<str:repo_name>/<int:pull_number>/try_jobs",
PullRequestTryPushAPIView.as_view(),
name="api-try-job-pull-request",
),
path(
"api/pulls/<str:repo_name>/<int:number>/checks",
PullRequestChecksAPIView.as_view(),
Expand Down