Skip to content

Commit

Permalink
feat: support auto-merge, handle settings that require Web UI, this f…
Browse files Browse the repository at this point in the history
…ixes #208 and #110
  • Loading branch information
netomi committed Mar 18, 2024
1 parent 7501d7a commit e294177
Show file tree
Hide file tree
Showing 29 changed files with 490 additions and 66 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@

### Added

- Added support for auto-merging of PRs under certain conditions. ([#110](https://github.com/eclipse-csi/otterdog/issues/110))
- Added handling for settings that require access to the Web UI. ([#208](https://github.com/eclipse-csi/otterdog/issues/208))
- Added support for repository setting `private_vulnerability_reporting_enabled`. ([#205](https://github.com/eclipse-csi/otterdog/issues/205))
- Added a graphql based query interface to the dashboard. ([#204](https://github.com/eclipse-csi/otterdog/issues/204))


## [0.5.0] - 05/03/2024

Note: this version includes lots of additions and changes related to the GitHub App mode which are not
Expand Down
1 change: 1 addition & 0 deletions DEPENDENCIES
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ pypi/pypi/-/python-dotenv/1.0.1
pypi/pypi/-/pyyaml/6.0.1
pypi/pypi/-/pyyaml-env-tag/0.1
pypi/pypi/-/quart/0.19.4
pypi/pypi/-/quart-flask-patch/0.3.0
pypi/pypi/-/quart-redis/2.0.0
pypi/pypi/-/redis/4.6.0
pypi/pypi/-/referencing/0.34.0
Expand Down
4 changes: 0 additions & 4 deletions docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ RUN poetry config virtualenvs.in-project true && \
poetry build && \
poetry install --only-root

WORKDIR /app/.venv/lib/python3.12/site-packages
COPY ./docker/odmantic.patch ./
RUN patch -p1 < odmantic.patch

# create the final image having python3.12 as base
FROM python:3.12.2-slim

Expand Down
17 changes: 0 additions & 17 deletions docker/odmantic.patch

This file was deleted.

4 changes: 4 additions & 0 deletions otterdog/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@
# which is available at http://www.eclipse.org/legal/epl-v20.html
# SPDX-License-Identifier: EPL-2.0
# *******************************************************************************

import importlib.metadata

__version__ = importlib.metadata.version(__name__)
8 changes: 2 additions & 6 deletions otterdog/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
# *******************************************************************************

import asyncio
import importlib.metadata
import sys
import traceback
from typing import Any

import click
from click.shell_completion import CompletionItem

from . import __version__
from .config import OtterdogConfig
from .operations import Operation
from .operations.apply import ApplyOperation
Expand All @@ -37,10 +37,6 @@
from .utils import IndentingPrinter, init, is_debug_enabled, print_error

_CONFIG_FILE = "otterdog.json"

_DISTRIBUTION_METADATA = importlib.metadata.metadata("otterdog")
_VERSION = _DISTRIBUTION_METADATA["Version"]

_CONTEXT_SETTINGS = dict(help_option_names=["-h", "--help"], max_content_width=120)

_CONFIG: OtterdogConfig | None = None
Expand Down Expand Up @@ -122,7 +118,7 @@ def invoke(self, ctx: click.Context) -> Any:


@click.group(context_settings=_CONTEXT_SETTINGS)
@click.version_option(version=_VERSION, prog_name="otterdog.sh")
@click.version_option(version=__version__, prog_name="otterdog.sh")
def cli():
"""
Managing GitHub organizations at scale.
Expand Down
20 changes: 20 additions & 0 deletions otterdog/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,20 @@ def of_changes(
LivePatchType.CHANGE, expected_object, current_object, changes, parent_object, forced_update, fn
)

def requires_web_ui(self) -> bool:
match self.patch_type:
case LivePatchType.ADD:
assert self.expected_object is not None
return self.expected_object.requires_web_ui()

case LivePatchType.REMOVE:
return False

case LivePatchType.CHANGE:
assert self.expected_object is not None
assert self.changes is not None
return self.expected_object.changes_require_web_ui(self.changes)

def requires_secrets(self) -> bool:
match self.patch_type:
case LivePatchType.ADD:
Expand Down Expand Up @@ -437,6 +451,12 @@ def to_model_dict(self, for_diff: bool = False, include_nested_models: bool = Fa

return result

def requires_web_ui(self) -> bool:
return False

def changes_require_web_ui(self, changes: dict[str, Change]) -> bool:
return False

def contains_secrets(self) -> bool:
return False

Expand Down
5 changes: 5 additions & 0 deletions otterdog/models/organization_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,11 @@ async def get_mapping_to_provider(
def get_jsonnet_template_function(self, jsonnet_config: JsonnetConfig, extend: bool) -> str | None:
return None

def changes_require_web_ui(self, changes: dict[str, Change]) -> bool:
from otterdog.providers.github import is_org_settings_key_retrieved_via_web_ui

return any(map(lambda key: is_org_settings_key_retrieved_via_web_ui(key), changes.keys()))

def to_jsonnet(
self,
printer: IndentingPrinter,
Expand Down
32 changes: 15 additions & 17 deletions otterdog/providers/github/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,22 @@

_ORG_SETTINGS_SCHEMA = json.loads(files(resources).joinpath("schemas/settings.json").read_text())

# collect supported rest api keys
_SETTINGS_RESTAPI_KEYS = {k for k, v in _ORG_SETTINGS_SCHEMA["properties"].items() if v.get("provider") == "restapi"}
# collect supported web interface keys
_SETTINGS_WEB_KEYS = {k for k, v in _ORG_SETTINGS_SCHEMA["properties"].items() if v.get("provider") == "web"}
# TODO: make this cleaner
_SETTINGS_WEB_KEYS.add("discussion_source_repository_id")


def is_org_settings_key_retrieved_via_web_ui(key: str) -> bool:
return key in _SETTINGS_WEB_KEYS


class GitHubProvider:
def __init__(self, credentials: Credentials | None):
self._credentials = credentials

self._settings_schema = _ORG_SETTINGS_SCHEMA
# collect supported rest api keys
self._settings_restapi_keys = {
k for k, v in self._settings_schema["properties"].items() if v.get("provider") == "restapi"
}

# collect supported web interface keys
self._settings_web_keys = {
k for k, v in self._settings_schema["properties"].items() if v.get("provider") == "web"
}
# TODO: make this cleaner
self._settings_web_keys.add("discussion_source_repository_id")

if credentials is not None:
self._init_clients()

Expand Down Expand Up @@ -73,13 +71,13 @@ async def update_content(

async def get_org_settings(self, org_id: str, included_keys: set[str], no_web_ui: bool) -> dict[str, Any]:
# first, get supported settings via the rest api.
required_rest_keys = {x for x in included_keys if x in self._settings_restapi_keys}
required_rest_keys = {x for x in included_keys if x in _SETTINGS_RESTAPI_KEYS}
merged_settings = await self.rest_api.org.get_settings(org_id, required_rest_keys)

# second, get settings only accessible via the web interface and merge
# them with the other settings, unless --no-web-ui is specified.
if not no_web_ui:
required_web_keys = {x for x in included_keys if x in self._settings_web_keys}
required_web_keys = {x for x in included_keys if x in _SETTINGS_WEB_KEYS}
if len(required_web_keys) > 0:
web_settings = await self.web_client.get_org_settings(org_id, required_web_keys)
merged_settings.update(web_settings)
Expand All @@ -95,9 +93,9 @@ async def update_org_settings(self, org_id: str, settings: dict[str, Any]) -> No
# split up settings to be updated whether they need be updated
# via rest api or web interface.
for k, v in sorted(settings.items()):
if k in self._settings_restapi_keys:
if k in _SETTINGS_RESTAPI_KEYS:
rest_fields[k] = v
elif k in self._settings_web_keys:
elif k in _SETTINGS_WEB_KEYS:
web_fields[k] = v
else:
utils.print_warn(f"encountered unknown field '{k}' during update, ignoring")
Expand Down
39 changes: 39 additions & 0 deletions otterdog/providers/github/rest/pull_request_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,42 @@ async def get_pull_requests(
except GitHubException as ex:
tb = ex.__traceback__
raise RuntimeError(f"failed retrieving pull requests:\n{ex}").with_traceback(tb)

async def get_reviews(self, org_id: str, repo_name: str, pull_request_number: str) -> list[dict[str, Any]]:
print_debug(f"getting reviews for pull request #{pull_request_number} from repo '{org_id}/{repo_name}'")

try:
return await self.requester.request_paged_json(
"GET", f"/repos/{org_id}/{repo_name}/pulls/{pull_request_number}/reviews"
)
except GitHubException as ex:
tb = ex.__traceback__
raise RuntimeError(f"failed retrieving pull request reviews:\n{ex}").with_traceback(tb)

async def merge(
self,
org_id: str,
repo_name: str,
pull_request_number: str,
commit_message: str | None = None,
merge_method: str = "squash",
) -> bool:
print_debug(f"merging pull request #{pull_request_number} from repo '{org_id}/{repo_name}'")

try:
data = {
"merge_method": merge_method,
}

if commit_message is not None:
data.update({"commit_message": commit_message})

response = await self.requester.request_json(
"PUT",
f"/repos/{org_id}/{repo_name}/pulls/{pull_request_number}/merge",
data=data,
)
return response["merged"]
except GitHubException as ex:
tb = ex.__traceback__
raise RuntimeError(f"failed merging pull request:\n{ex}").with_traceback(tb)
1 change: 1 addition & 0 deletions otterdog/webapp/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from importlib.util import find_spec
from typing import TYPE_CHECKING

import quart_flask_patch # type: ignore # noqa: F401
from quart import Quart
from quart.json.provider import DefaultJSONProvider
from quart_redis import RedisHandler # type: ignore
Expand Down
12 changes: 11 additions & 1 deletion otterdog/webapp/db/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ class PullRequestModel(Model):

valid: Optional[bool] = None
in_sync: Optional[bool] = None
requires_manual_apply: bool = False
requires_manual_apply: Optional[bool] = None
supports_auto_merge: Optional[bool] = None
has_required_approval: Optional[bool] = None

created_at: datetime = Field(index=True)
updated_at: datetime = Field(index=True)
Expand All @@ -113,6 +115,14 @@ class PullRequestModel(Model):
],
}

def can_be_automerged(self) -> bool:
return (
self.valid is True
and self.in_sync is True
and self.supports_auto_merge is True
and self.has_required_approval is True
)


class StatisticsModel(Model):
project_name: str = Field(primary_field=True)
Expand Down
11 changes: 10 additions & 1 deletion otterdog/webapp/db/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,10 @@ async def update_or_create_pull_request(
valid: bool | None = None,
in_sync: bool | None = None,
requires_manual_apply: bool | None = None,
supports_auto_merge: bool | None = None,
has_required_approvals: bool | None = None,
apply_status: ApplyStatus | None = None,
) -> None:
) -> PullRequestModel:
pull_request_status = PullRequestStatus[pull_request.get_pr_status()]

pr_model = await find_pull_request(owner, repo, pull_request.number)
Expand Down Expand Up @@ -344,7 +346,14 @@ async def update_or_create_pull_request(
if requires_manual_apply is not None:
pr_model.requires_manual_apply = requires_manual_apply

if supports_auto_merge is not None:
pr_model.supports_auto_merge = supports_auto_merge

if has_required_approvals is not None:
pr_model.has_required_approval = has_required_approvals

await update_pull_request(pr_model)
return pr_model


async def update_pull_request(pull_request: PullRequestModel) -> None:
Expand Down
4 changes: 2 additions & 2 deletions otterdog/webapp/home/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ async def route_default():

@blueprint.route("/robots.txt")
@blueprint.route("/favicon.ico")
def static_from_root():
return send_from_directory(current_app.static_folder, request.path[1:])
async def static_from_root():
return await send_from_directory(current_app.static_folder, request.path[1:])


@blueprint.route("/index")
Expand Down
15 changes: 15 additions & 0 deletions otterdog/webapp/tasks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
# SPDX-License-Identifier: EPL-2.0
# *******************************************************************************

from __future__ import annotations

import contextlib
from abc import ABC, abstractmethod
from collections.abc import AsyncIterator
Expand All @@ -14,6 +16,7 @@
from typing import Any, Generic, Protocol, TypeVar

import aiofiles
from quart import current_app

from otterdog.config import OrganizationConfig
from otterdog.providers.github import GitHubProvider, GraphQLClient
Expand Down Expand Up @@ -216,6 +219,18 @@ async def minimize_outdated_comments(
if bool(is_minimized) is False and matching_header in body:
await graphql_api.minimize_comment(comment_id, "OUTDATED")

def schedule_automerge_task(self, org_id: str, repo_name: str, pull_request_number: int) -> None:
from .auto_merge_comment import AutoMergeCommentTask

current_app.add_background_task(
AutoMergeCommentTask(
self.installation_id,
org_id,
repo_name,
pull_request_number,
)
)

async def _cleanup(self) -> None:
if self.__rest_api is not None:
await self.__rest_api.close()
Expand Down
2 changes: 1 addition & 1 deletion otterdog/webapp/tasks/apply_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ async def _execute(self) -> ApplyResult:
operation_result = await operation.execute(org_config)
apply_result.apply_output = output.getvalue()
apply_result.apply_success = operation_result == 0
apply_result.partial = self._pr_model.requires_manual_apply
apply_result.partial = self._pr_model.requires_manual_apply is True
except Exception as ex:
self.logger.exception("exception during apply", exc_info=ex)

Expand Down
Loading

0 comments on commit e294177

Please sign in to comment.