Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
85fba5e
github: add GitHubAPI, GitHubAPIClient classes (bug 1989637) (#536)
zzzeid Oct 22, 2025
4b5285b
github: add pull request view, url, template (bug 1989963) (#553)
zzzeid Oct 22, 2025
7c0d34d
github: add pull request endpoint (bug 1991125) (#582)
zzzeid Oct 22, 2025
dc3ebcb
pull_request: add more functionality to end points and ui (bug 199112…
zzzeid Oct 28, 2025
222bef8
github: add write methods (bug 1989960) (#611)
zzzeid Oct 28, 2025
0e2ec95
landing_worker: add comment + close pr functionality (bug 1994736) (#…
zzzeid Oct 28, 2025
7c1d6a8
pull_requests: add landing job timeline (bug 1995378) (#616)
zzzeid Oct 28, 2025
b8472c8
GitSCM/GitHubAPI: move all github logic to dedicated class
shtrom Oct 14, 2025
c9ec66a
github: add GitHub class for basic URL and token manipulation (bug 19…
shtrom Oct 22, 2025
b57ce51
utils: move URL_USERINFO_RE to const (bug 1995679)
shtrom Oct 27, 2025
3d9712a
GitHubAPIClient: scope GET requests to the chosen repo (#608)
shtrom Oct 29, 2025
4563f49
fixup! GitSCM/GitHubAPI: move all github logic to dedicated class
shtrom Oct 30, 2025
ad30b6c
Merge remote-tracking branch 'origin/zeid/bug-1989635-github-pr-pilot…
shtrom Oct 30, 2025
e49bb55
tests: run utils test suite (bug 1996744) (#643)
shtrom Oct 28, 2025
4493dee
tests: add tests for GitHub class
shtrom Oct 30, 2025
12b2be3
Update src/lando/utils/github.py
shtrom Oct 31, 2025
dc6994e
Merge branch 'zeid/bug-1989635-github-pr-pilot' into no-bug/github-ap…
shtrom Oct 31, 2025
09000ad
github: make repo_url parsing more resilient and predictable
shtrom Oct 31, 2025
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
13 changes: 0 additions & 13 deletions pyproject.toml
Comment thread
shtrom marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,3 @@ where = ["src"]
[tool.pytest.ini_options]
DJANGO_SETTINGS_MODULE = "lando.test_settings"
addopts = "--cov --cov-report html"

testpaths = [
"src/lando/api",
"src/lando/dockerflow",
"src/lando/headless_api",
"src/lando/main/tests",
"src/lando/pulse/tests",
"src/lando/pushlog/tests",
"src/lando/tests",
"src/lando/treestatus/tests",
"src/lando/try_api/tests",
"src/lando/ui/tests",
]
10 changes: 10 additions & 0 deletions src/lando/api/legacy/workers/landing_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
)
from lando.pushlog.pushlog import PushLog, PushLogForRepo
from lando.utils.config import read_lando_config
from lando.utils.github import GitHubAPIClient
from lando.utils.tasks import phab_trigger_repo_update

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -143,6 +144,15 @@ def run_job(self, job: LandingJob) -> bool:
job.set_landed_commit_ids()
job.transition_status(JobAction.LAND, commit_id=commit_id)

if job.is_pull_request_job:
# TODO: move this to different method, and retry if needed.
# NOTE: This may need to happen on the revision-level when stack support is added.
pull_number = job.revisions.first().pull_number
message = f"Pull request closed by commit {commit_id}"
client = GitHubAPIClient(job.target_repo.url)
client.add_comment_to_pull_request(pull_number, message)
client.close_pull_request(pull_number)

mots_path = Path(repo.path) / "mots.yaml"
if mots_path.exists():
logger.info(f"{mots_path} found, setting reviewer data.")
Expand Down
92 changes: 91 additions & 1 deletion src/lando/api/views.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import json
from collections import defaultdict
from datetime import datetime
from functools import wraps
from typing import Callable

Expand All @@ -9,15 +11,30 @@
from django.views import View
from django.views.decorators.csrf import csrf_exempt

from lando.main.models import CommitMap
from lando.main.models import (
CommitMap,
JobStatus,
LandingJob,
Repo,
Revision,
add_revisions_to_job,
)
from lando.main.models.landing_job import get_jobs_for_pull
from lando.main.models.revision import DiffWarning, DiffWarningStatus
from lando.main.scm import (
SCM_TYPE_GIT,
SCM_TYPE_HG,
)
from lando.utils.github import GitHubAPIClient, PullRequest
from lando.utils.phabricator import get_phabricator_client


class APIView(View):
"""A base class for API views."""

pass


def phabricator_api_key_required(func: callable) -> Callable:
"""A simple wrapper that checks for a valid Phabricator API token."""

Expand Down Expand Up @@ -148,3 +165,76 @@ class hg2gitCommitMapView(CommitMapBaseView):
"""Return corresponding CommitMap given an hg hash."""

scm = SCM_TYPE_HG


class LandingJobPullRequestAPIView(View):
"""Handle pull request landing jobs in the API."""

def get(
self, request: WSGIRequest, repo_name: int, pull_number: int
) -> JsonResponse:
"""Return the status of a pull request based on landing job counts."""

target_repo = Repo.objects.get(name=repo_name)
landing_jobs = get_jobs_for_pull(target_repo, pull_number)
landing_jobs_by_status = defaultdict(list)
for landing_job in landing_jobs:
landing_jobs_by_status[landing_job.status].append(landing_job.id)

status = None
# Return the first encountered status in this list.
for _status in [
JobStatus.LANDED,
JobStatus.CREATED,
JobStatus.SUBMITTED,
JobStatus.IN_PROGRESS,
JobStatus.FAILED,
]:
if landing_jobs_by_status[_status]:
status = str(_status).lower()
break

return JsonResponse({"status": status}, status=200)

def post(
self, request: WSGIRequest, repo_name: int, pull_number: int
) -> JsonResponse:
"""Create a new landing job for a pull request."""

class Form(forms.Form):
"""Simple form to get clean some fields."""

head_sha = forms.CharField()
# TODO: use this for verification later, see bug 1996571.
# base_ref = forms.CharField()

target_repo = Repo.objects.get(name=repo_name)
client = GitHubAPIClient(target_repo.url)
ldap_username = request.user.email
pull_request = PullRequest(client.get_pull_request(pull_number))
form = Form(json.loads(request.body))

if not form.is_valid():
return JsonResponse(form.errors, 400)

# TODO: this does not work with binary data, must use patch instead.
# See bug 1993047.
diff = client.get_diff(pull_number)
job = LandingJob.objects.create(
target_repo=target_repo, requester_email=ldap_username
)
revision = Revision.objects.create(pull_number=pull_request.number)
patch_data = {
# See bug 1995006 (to actually parse authorship info). Use placeholder for now.
"author_name": "Author Name",
"author_email": "Author Email <email@example.org>",
"commit_message": pull_request.title,
"timestamp": int(datetime.now().timestamp()),
}
revision.set_patch(diff, patch_data)
revision.save()
add_revisions_to_job([revision], job)
job.status = JobStatus.SUBMITTED
job.save()

return JsonResponse({"id": job.id}, status=201)
18 changes: 18 additions & 0 deletions src/lando/main/migrations/0033_revision_pull_number.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 5.2.5 on 2025-10-28 13:40

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("main", "0032_revisionlandingjob_commit_id"),
]

operations = [
migrations.AddField(
model_name="revision",
name="pull_number",
field=models.IntegerField(blank=True, null=True),
),
]
16 changes: 16 additions & 0 deletions src/lando/main/models/landing_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from mots.directory import Directory

from lando.main.models.jobs import BaseJob
from lando.main.models.repo import Repo
from lando.main.models.revision import Revision, RevisionLandingJob

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -47,6 +48,11 @@ class LandingJob(BaseJob):
Revision, through="RevisionLandingJob", related_name="landing_jobs"
)

@property
def is_pull_request_job(self) -> bool:
"""Return True if all revisions in the landing job have a pull_number set."""
return not self.revisions.filter(pull_number__isnull=True).exists()

@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 @@ -265,3 +271,13 @@ def add_revisions_to_job(revisions: list[Revision], job: LandingJob):
"""Given an existing job, add and sort provided revisions."""
job.add_revisions(revisions)
job.sort_revisions(revisions)


def get_jobs_for_pull(target_repo: Repo, pull_number: int) -> QuerySet[LandingJob]:
"""Given a target repo and a pull number, return all landing jobs."""
revisions = Revision.objects.filter(
landing_jobs__target_repo=target_repo, pull_number=pull_number
)
return LandingJob.objects.filter(unsorted_revisions__in=revisions).order_by(
"-created_at"
)
5 changes: 5 additions & 0 deletions src/lando/main/models/repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,11 @@ def _github_repo_url(self) -> str | None:
if self.is_github:
return self.url.removesuffix(".git")

@property
def _github_repo_org(self) -> str | None:
if self.is_github:
return self._github_repo_url.split("/")[-2]

@property
def git_repo_name(self) -> str:
"""Provide the bare name of the Git repo."""
Expand Down
3 changes: 3 additions & 0 deletions src/lando/main/models/revision.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class Revision(BaseModel):
# does not track all diffs.
diff_id = models.IntegerField(blank=True, null=True)

# GitHub pull request number, if this is a pull request.
pull_number = models.IntegerField(blank=True, null=True)

# The actual patch with Mercurial metadata format.
patch = models.TextField(blank=True, default="")

Expand Down
72 changes: 5 additions & 67 deletions src/lando/main/scm/git.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import asyncio
import io
import logging
import os
Expand All @@ -11,8 +10,6 @@
from pathlib import Path
from typing import Any

from django.conf import settings
from simple_github import AppAuth, AppInstallationAuth
from typing_extensions import override

from lando.main.scm.commit import CommitData
Expand All @@ -25,6 +22,8 @@
)
from lando.main.scm.helpers import GitPatchHelper, PatchHelper
from lando.settings import LANDO_USER_EMAIL, LANDO_USER_NAME
from lando.utils.const import URL_USERINFO_RE
from lando.utils.github import GitHub

from .abstract_scm import AbstractSCM

Expand All @@ -36,24 +35,6 @@
ENV_COMMITTER_NAME = "GIT_COMMITTER_NAME"
ENV_COMMITTER_EMAIL = "GIT_COMMITTER_EMAIL"

# From RFC-3986 [0]:
#
# userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
#
# unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
# pct-encoded = "%" HEXDIG HEXDIG
# sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
# / "*" / "+" / "," / ";" / "=
#
# [0] https://www.rfc-editor.org/rfc/rfc3986
URL_USERINFO_RE = re.compile(
"(?P<userinfo>[-A-Za-z0-9:._~%!$&'*()*+;=]*:[-A-Za-z0-9:._~%!$&'*()*+;=]*@)",
flags=re.MULTILINE,
)
GITHUB_URL_RE = re.compile(
f"https://{URL_USERINFO_RE.pattern}?github.com/(?P<owner>[-A-Za-z0-9]+)/(?P<repo>[^/]+)"
)


class GitSCM(AbstractSCM):
"""An implementation of the AbstractVCS for Git, for use by the Repo and LandingWorkers."""
Expand Down Expand Up @@ -112,31 +93,14 @@ def push(
tags: list[str] | None = None,
):
"""Push local code to the remote repository."""

push_command = ["push"]

if force_push:
push_command += ["--force"]

if match := re.match(GITHUB_URL_RE, push_path):
# We only fetch a token if no authentication is explicitly specified in
# the push_url.
if not match["userinfo"]:
logger.info(
"Obtaining fresh GitHub token repo",
extra={
"push_path": push_path,
"repo_name": match["repo"],
"repo_owner": match["owner"],
},
)

owner = match["owner"]
repo = match["repo"]
repo_name = repo.removesuffix(".git")

token = self._get_github_token(owner, repo_name)
if token:
push_path = f"https://git:{token}@github.com/{owner}/{repo}"
if GitHub.is_supported_url(push_path):
push_path = GitHub(push_path).authenticated_url

push_command += [push_path]

Expand All @@ -152,32 +116,6 @@ def push(

self._git_run(*push_command, cwd=self.path)

@staticmethod
def _get_github_token(repo_owner: str, repo_name: str) -> str | None:
"""Obtain a fresh GitHub token to push to the specified repo.

This relies on GITHUB_APP_ID and GITHUB_APP_PRIVKEY to be set in the
settings. Returns None if those are missing.

The app with ID GITHUB_APP_ID needs to be enabled for the target repo.

"""
app_id = settings.GITHUB_APP_ID
private_key = settings.GITHUB_APP_PRIVKEY

if not app_id or not private_key:
logger.warning(
f"Missing GITHUB_APP_ID or GITHUB_APP_PRIVKEY to authenticate against GitHub repo {repo_owner}/{repo_name}",
)
return None

app_auth = AppAuth(
app_id,
private_key,
)
session = AppInstallationAuth(app_auth, repo_owner, repositories=[repo_name])
return asyncio.run(session.get_token())

def last_commit_for_path(self, path: str) -> str:
"""Find last commit to touch a path."""
command = ["log", "--max-count=1", "--format=%H", "--", path]
Expand Down
Loading