Skip to content

Commit

Permalink
feat!: stop syncing labels from labels.yaml
Browse files Browse the repository at this point in the history
The repo_checks[1] tool now does this, and we don't want two
systems simultaneously trying to create or delete labels.

Note that the bot will still *add* and *delete* a handful of labels
from *PRs* (`open-source-pull-request`, `blended`, etc).
What this commit changes is that the bot will not longer *create*
nor *delete* labels from the *entire repository*.

The `labels.yaml` file is now unused and can be safely deleted from any
openedx-webhooks data repositories.

[1] https://github.com/openedx/repo-tools/tree/master/edx_repo_tools/repo_checks

Part of: openedx#218
  • Loading branch information
kdmccormick committed Jun 17, 2023
1 parent 51e8a9b commit 93584d6
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 192 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.. A new scriv changelog fragment.
- Stopped the bot from updating a repository's labels based on ``labels.yaml``, as this is now handled by the `repo_checks <https://github.com/openedx/repo-tools/tree/master/edx_repo_tools/repo_checks>`_ tool. The ``labels.yaml`` file is now unused and can be safely deleted from any openedx-webhooks data repositories.
4 changes: 1 addition & 3 deletions openedx_webhooks/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,14 @@ def get_people_file():
people[p].update(people_data_yaml[p])
return people


def get_orgs_file():
orgs = _read_yaml_data_file("orgs.yaml")
for org_data in list(orgs.values()):
if "name" in org_data:
orgs[org_data["name"]] = org_data
return orgs

def get_labels_file():
return _read_yaml_data_file("labels.yaml")

def get_person_certain_time(person: Dict, certain_time: datetime.datetime) -> Dict:
"""
Return person data structure for a particular time
Expand Down
4 changes: 1 addition & 3 deletions openedx_webhooks/jira_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from urlobject import URLObject

from openedx_webhooks.auth import get_github_session, get_jira_session
from openedx_webhooks.tasks.github_work import get_repo_labels, synchronize_labels
from openedx_webhooks.tasks.github_work import get_repo_labels
from openedx_webhooks.utils import (
jira_get, jira_paginated_get, sentry_extra_context,
github_pr_num, github_pr_url, github_pr_repo,
Expand Down Expand Up @@ -272,8 +272,6 @@ def issue_updated():
fail_msg += ' {0}'.format(event["issue"]["fields"]["issuetype"])
raise Exception(fail_msg)

synchronize_labels(pr_repo)

repo_labels = get_repo_labels(repo=pr_repo)
repo_labels_lower = {name.lower() for name in repo_labels}

Expand Down
39 changes: 1 addition & 38 deletions openedx_webhooks/tasks/github_work.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,8 @@
from typing import Any, Dict

from openedx_webhooks.auth import get_github_session
from openedx_webhooks.info import get_labels_file
from openedx_webhooks.tasks import logger
from openedx_webhooks.utils import (
log_check_response,
memoize_timed,
paginated_get,
)
from openedx_webhooks.utils import paginated_get


def get_repo_labels(repo: str) -> Dict[str, Dict[str, Any]]:
Expand All @@ -20,35 +15,3 @@ def get_repo_labels(repo: str) -> Dict[str, Dict[str, Any]]:
repo_labels = {lbl["name"]: lbl for lbl in paginated_get(url, session=get_github_session())}
return repo_labels


@memoize_timed(minutes=15)
def synchronize_labels(repo: str) -> None:
"""Ensure the labels in `repo` match the specs in openedx-webhooks-data/labels.yaml"""

url = f"/repos/{repo}/labels"
repo_labels = get_repo_labels(repo)
desired_labels = get_labels_file()
for name, label_data in desired_labels.items():
if label_data.get("delete", False):
# A label that should not exist in the repo.
if name in repo_labels:
logger.info(f"Deleting label {name} from {repo}")
resp = get_github_session().delete(f"{url}/{name}")
log_check_response(resp)
else:
# A label that should exist in the repo.
label_data["name"] = name
if name in repo_labels:
repo_label = repo_labels[name]
color_differs = repo_label["color"] != label_data["color"]
repo_desc = repo_label.get("description", "") or ""
desired_desc = label_data.get("description", "") or ""
desc_differs = repo_desc != desired_desc
if color_differs or desc_differs:
logger.info(f"Updating label {name} in {repo}")
resp = get_github_session().patch(f"{url}/{name}", json=label_data)
log_check_response(resp)
else:
logger.info(f"Adding label {name} to {repo}")
resp = get_github_session().post(url, json=label_data)
log_check_response(resp)
5 changes: 0 additions & 5 deletions openedx_webhooks/tasks/pr_tracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,6 @@ def fix_ospr(self) -> None:
desired=json_safe_dict(self.desired),
)

self.actions.synchronize_labels(repo=self.prid.full_name)

comment_kwargs = {}

make_issue = False
Expand Down Expand Up @@ -785,9 +783,6 @@ def initial_state(self, *, current: Dict, desired: Dict) -> None:
Does nothing when really fixing, but captures information for dry runs.
"""

def synchronize_labels(self, *, repo: str) -> None:
github_work.synchronize_labels(repo)

def create_ospr_issue(
self, *,
pr_url: str,
Expand Down
11 changes: 0 additions & 11 deletions tests/repo_data/openedx/openedx-webhooks-data/labels.yaml

This file was deleted.

31 changes: 7 additions & 24 deletions tests/test_pull_request_opened.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@
pytestmark = pytest.mark.flaky_github


@pytest.fixture
def sync_labels_fn(mocker):
"""A patch on synchronize_labels"""
return mocker.patch("openedx_webhooks.tasks.github_work.synchronize_labels")

def close_and_reopen_pr(pr):
"""For testing re-opening, close the pr, process it, then re-open it."""
pr.close(merge=False)
Expand Down Expand Up @@ -96,7 +91,7 @@ def test_pr_opened_by_bot(fake_github, fake_jira):
assert pull_request_projects(pr.as_json()) == set()


def test_external_pr_opened_no_cla(has_jira, sync_labels_fn, fake_github, fake_jira):
def test_external_pr_opened_no_cla(has_jira, fake_github, fake_jira):
# No CLA, because this person is not in people.yaml
fake_github.make_user(login="new_contributor", name="Newb Contributor")
pr = fake_github.make_pull_request(owner="openedx", repo="edx-platform", user="new_contributor")
Expand Down Expand Up @@ -128,9 +123,6 @@ def test_external_pr_opened_no_cla(has_jira, sync_labels_fn, fake_github, fake_j
assert issue_id is None
assert len(fake_jira.issues) == 0

# Check that we synchronized labels.
sync_labels_fn.assert_called_once_with("openedx/edx-platform")

# Check the GitHub comment that was created.
pr_comments = pr.list_comments()
assert len(pr_comments) == 1
Expand Down Expand Up @@ -167,7 +159,7 @@ def test_external_pr_opened_no_cla(has_jira, sync_labels_fn, fake_github, fake_j
assert len(fake_jira.issues) == 0


def test_external_pr_opened_with_cla(has_jira, sync_labels_fn, fake_github, fake_jira):
def test_external_pr_opened_with_cla(has_jira, fake_github, fake_jira):
pr = fake_github.make_pull_request(owner="openedx", repo="some-code", user="tusbar", number=11235)
prj = pr.as_json()

Expand Down Expand Up @@ -197,9 +189,6 @@ def test_external_pr_opened_with_cla(has_jira, sync_labels_fn, fake_github, fake
assert issue_id is None
assert len(fake_jira.issues) == 0

# Check that we synchronized labels.
sync_labels_fn.assert_called_once_with("openedx/some-code")

# Check the GitHub comment that was created.
pr_comments = pr.list_comments()
assert len(pr_comments) == 1
Expand Down Expand Up @@ -238,7 +227,7 @@ def test_external_pr_opened_with_cla(has_jira, sync_labels_fn, fake_github, fake
assert len(fake_jira.issues) == 0


def test_psycho_reopening(sync_labels_fn, fake_github, fake_jira):
def test_psycho_reopening(fake_github, fake_jira):
# Check that close/re-open/close/re-open etc will properly track the jira status.
pr = fake_github.make_pull_request(owner="openedx", repo="some-code", user="tusbar", number=11235)
prj = pr.as_json()
Expand All @@ -257,7 +246,7 @@ def test_psycho_reopening(sync_labels_fn, fake_github, fake_jira):
assert issue.status == status


def test_core_committer_pr_opened(has_jira, sync_labels_fn, fake_github, fake_jira):
def test_core_committer_pr_opened(has_jira, fake_github, fake_jira):
pr = fake_github.make_pull_request(user="felipemontoya", owner="openedx", repo="edx-platform")
prj = pr.as_json()

Expand Down Expand Up @@ -287,9 +276,6 @@ def test_core_committer_pr_opened(has_jira, sync_labels_fn, fake_github, fake_ji
assert issue_id is None
assert len(fake_jira.issues) == 0

# Check that we synchronized labels.
sync_labels_fn.assert_called_once_with("openedx/edx-platform")

# Check the GitHub comment that was created.
pr_comments = pr.list_comments()
assert len(pr_comments) == 1
Expand All @@ -311,7 +297,7 @@ def test_core_committer_pr_opened(has_jira, sync_labels_fn, fake_github, fake_ji
assert pull_request_projects(pr.as_json()) == {settings.GITHUB_OSPR_PROJECT}


def test_old_core_committer_pr_opened(sync_labels_fn, fake_github, fake_jira):
def test_old_core_committer_pr_opened(fake_github, fake_jira):
# No-one was a core committer before June 2020.
# This test only asserts the core-committer things, that they are not cc.
pr = fake_github.make_pull_request(
Expand Down Expand Up @@ -356,7 +342,7 @@ def test_old_core_committer_pr_opened(sync_labels_fn, fake_github, fake_jira):
pytest.param(False, id="epic:no"),
pytest.param(True, id="epic:yes"),
])
def test_blended_pr_opened_with_cla(with_epic, has_jira, sync_labels_fn, fake_github, fake_jira):
def test_blended_pr_opened_with_cla(with_epic, has_jira, fake_github, fake_jira):
pr = fake_github.make_pull_request(owner="openedx", repo="some-code", user="tusbar", title="[BD-34] Something good")
prj = pr.as_json()
total_issues = 0
Expand Down Expand Up @@ -401,9 +387,6 @@ def test_blended_pr_opened_with_cla(with_epic, has_jira, sync_labels_fn, fake_gi
assert issue_id is None
assert len(fake_jira.issues) == total_issues

# Check that we synchronized labels.
sync_labels_fn.assert_called_once_with("openedx/some-code")

# Check the GitHub comment that was created.
pr_comments = pr.list_comments()
assert len(pr_comments) == 1
Expand Down Expand Up @@ -825,7 +808,7 @@ def test_draft_pr_opened(pr_type, jira_got_fiddled, has_jira, fake_github, fake_
assert initial_status.lower() in pr.labels


def test_handle_closed_pr(is_merged, has_jira, sync_labels_fn, fake_github, fake_jira):
def test_handle_closed_pr(is_merged, has_jira, fake_github, fake_jira):
pr = fake_github.make_pull_request(user="tusbar", number=11237, state="closed", merged=is_merged)
prj = pr.as_json()
issue_id1, anything_happened = pull_request_changed(prj)
Expand Down
4 changes: 0 additions & 4 deletions tests/test_rescan.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ def test_rescan_repository_dry_run(rescannable_repo, fake_github, fake_jira, pul
102: [
"set_cla_status",
"initial_state",
"synchronize_labels",
"create_ospr_issue",
"update_labels_on_pull_request",
"add_comment_to_pull_request",
Expand All @@ -113,7 +112,6 @@ def test_rescan_repository_dry_run(rescannable_repo, fake_github, fake_jira, pul
106: [
"set_cla_status",
"initial_state",
"synchronize_labels",
"create_ospr_issue",
"update_labels_on_pull_request",
"add_comment_to_pull_request",
Expand All @@ -122,7 +120,6 @@ def test_rescan_repository_dry_run(rescannable_repo, fake_github, fake_jira, pul
108: [
"set_cla_status",
"initial_state",
"synchronize_labels",
"create_ospr_issue",
"transition_jira_issue",
"update_labels_on_pull_request",
Expand All @@ -132,7 +129,6 @@ def test_rescan_repository_dry_run(rescannable_repo, fake_github, fake_jira, pul
110: [
"set_cla_status",
"initial_state",
"synchronize_labels",
"transition_jira_issue",
"update_jira_issue",
"update_labels_on_pull_request",
Expand Down
104 changes: 0 additions & 104 deletions tests/test_synchronize_labels.py

This file was deleted.

0 comments on commit 93584d6

Please sign in to comment.