Skip to content

Commit 0cd5240

Browse files
committed
feat!: stop syncing labels from labels.yaml
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`, `core-committer`, etc). What this commit changes is that the bot will not longer *create* nor *delete* labels from the *entire repository*. The `label.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
1 parent 51e8a9b commit 0cd5240

File tree

9 files changed

+13
-192
lines changed

9 files changed

+13
-192
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
.. A new scriv changelog fragment.
2+
3+
- 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 ``label.yaml`` file is now unused and can be safely deleted from any openedx-webhooks data repositories.

openedx_webhooks/info.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,16 +121,14 @@ def get_people_file():
121121
people[p].update(people_data_yaml[p])
122122
return people
123123

124+
124125
def get_orgs_file():
125126
orgs = _read_yaml_data_file("orgs.yaml")
126127
for org_data in list(orgs.values()):
127128
if "name" in org_data:
128129
orgs[org_data["name"]] = org_data
129130
return orgs
130131

131-
def get_labels_file():
132-
return _read_yaml_data_file("labels.yaml")
133-
134132
def get_person_certain_time(person: Dict, certain_time: datetime.datetime) -> Dict:
135133
"""
136134
Return person data structure for a particular time

openedx_webhooks/jira_views.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from urlobject import URLObject
1212

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

275-
synchronize_labels(pr_repo)
276-
277275
repo_labels = get_repo_labels(repo=pr_repo)
278276
repo_labels_lower = {name.lower() for name in repo_labels}
279277

openedx_webhooks/tasks/github_work.py

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,8 @@
55
from typing import Any, Dict
66

77
from openedx_webhooks.auth import get_github_session
8-
from openedx_webhooks.info import get_labels_file
98
from openedx_webhooks.tasks import logger
10-
from openedx_webhooks.utils import (
11-
log_check_response,
12-
memoize_timed,
13-
paginated_get,
14-
)
9+
from openedx_webhooks.utils import paginated_get
1510

1611

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

23-
24-
@memoize_timed(minutes=15)
25-
def synchronize_labels(repo: str) -> None:
26-
"""Ensure the labels in `repo` match the specs in openedx-webhooks-data/labels.yaml"""
27-
28-
url = f"/repos/{repo}/labels"
29-
repo_labels = get_repo_labels(repo)
30-
desired_labels = get_labels_file()
31-
for name, label_data in desired_labels.items():
32-
if label_data.get("delete", False):
33-
# A label that should not exist in the repo.
34-
if name in repo_labels:
35-
logger.info(f"Deleting label {name} from {repo}")
36-
resp = get_github_session().delete(f"{url}/{name}")
37-
log_check_response(resp)
38-
else:
39-
# A label that should exist in the repo.
40-
label_data["name"] = name
41-
if name in repo_labels:
42-
repo_label = repo_labels[name]
43-
color_differs = repo_label["color"] != label_data["color"]
44-
repo_desc = repo_label.get("description", "") or ""
45-
desired_desc = label_data.get("description", "") or ""
46-
desc_differs = repo_desc != desired_desc
47-
if color_differs or desc_differs:
48-
logger.info(f"Updating label {name} in {repo}")
49-
resp = get_github_session().patch(f"{url}/{name}", json=label_data)
50-
log_check_response(resp)
51-
else:
52-
logger.info(f"Adding label {name} to {repo}")
53-
resp = get_github_session().post(url, json=label_data)
54-
log_check_response(resp)

openedx_webhooks/tasks/pr_tracking.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -417,8 +417,6 @@ def fix_ospr(self) -> None:
417417
desired=json_safe_dict(self.desired),
418418
)
419419

420-
self.actions.synchronize_labels(repo=self.prid.full_name)
421-
422420
comment_kwargs = {}
423421

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

788-
def synchronize_labels(self, *, repo: str) -> None:
789-
github_work.synchronize_labels(repo)
790-
791786
def create_ospr_issue(
792787
self, *,
793788
pr_url: str,

tests/repo_data/openedx/openedx-webhooks-data/labels.yaml

Lines changed: 0 additions & 11 deletions
This file was deleted.

tests/test_pull_request_opened.py

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,6 @@
2727
pytestmark = pytest.mark.flaky_github
2828

2929

30-
@pytest.fixture
31-
def sync_labels_fn(mocker):
32-
"""A patch on synchronize_labels"""
33-
return mocker.patch("openedx_webhooks.tasks.github_work.synchronize_labels")
34-
3530
def close_and_reopen_pr(pr):
3631
"""For testing re-opening, close the pr, process it, then re-open it."""
3732
pr.close(merge=False)
@@ -96,7 +91,7 @@ def test_pr_opened_by_bot(fake_github, fake_jira):
9691
assert pull_request_projects(pr.as_json()) == set()
9792

9893

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

131-
# Check that we synchronized labels.
132-
sync_labels_fn.assert_called_once_with("openedx/edx-platform")
133-
134126
# Check the GitHub comment that was created.
135127
pr_comments = pr.list_comments()
136128
assert len(pr_comments) == 1
@@ -167,7 +159,7 @@ def test_external_pr_opened_no_cla(has_jira, sync_labels_fn, fake_github, fake_j
167159
assert len(fake_jira.issues) == 0
168160

169161

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

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

200-
# Check that we synchronized labels.
201-
sync_labels_fn.assert_called_once_with("openedx/some-code")
202-
203192
# Check the GitHub comment that was created.
204193
pr_comments = pr.list_comments()
205194
assert len(pr_comments) == 1
@@ -238,7 +227,7 @@ def test_external_pr_opened_with_cla(has_jira, sync_labels_fn, fake_github, fake
238227
assert len(fake_jira.issues) == 0
239228

240229

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

259248

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

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

290-
# Check that we synchronized labels.
291-
sync_labels_fn.assert_called_once_with("openedx/edx-platform")
292-
293279
# Check the GitHub comment that was created.
294280
pr_comments = pr.list_comments()
295281
assert len(pr_comments) == 1
@@ -311,7 +297,7 @@ def test_core_committer_pr_opened(has_jira, sync_labels_fn, fake_github, fake_ji
311297
assert pull_request_projects(pr.as_json()) == {settings.GITHUB_OSPR_PROJECT}
312298

313299

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

404-
# Check that we synchronized labels.
405-
sync_labels_fn.assert_called_once_with("openedx/some-code")
406-
407390
# Check the GitHub comment that was created.
408391
pr_comments = pr.list_comments()
409392
assert len(pr_comments) == 1
@@ -825,7 +808,7 @@ def test_draft_pr_opened(pr_type, jira_got_fiddled, has_jira, fake_github, fake_
825808
assert initial_status.lower() in pr.labels
826809

827810

828-
def test_handle_closed_pr(is_merged, has_jira, sync_labels_fn, fake_github, fake_jira):
811+
def test_handle_closed_pr(is_merged, has_jira, fake_github, fake_jira):
829812
pr = fake_github.make_pull_request(user="tusbar", number=11237, state="closed", merged=is_merged)
830813
prj = pr.as_json()
831814
issue_id1, anything_happened = pull_request_changed(prj)

tests/test_rescan.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ def test_rescan_repository_dry_run(rescannable_repo, fake_github, fake_jira, pul
104104
102: [
105105
"set_cla_status",
106106
"initial_state",
107-
"synchronize_labels",
108107
"create_ospr_issue",
109108
"update_labels_on_pull_request",
110109
"add_comment_to_pull_request",
@@ -113,7 +112,6 @@ def test_rescan_repository_dry_run(rescannable_repo, fake_github, fake_jira, pul
113112
106: [
114113
"set_cla_status",
115114
"initial_state",
116-
"synchronize_labels",
117115
"create_ospr_issue",
118116
"update_labels_on_pull_request",
119117
"add_comment_to_pull_request",
@@ -122,7 +120,6 @@ def test_rescan_repository_dry_run(rescannable_repo, fake_github, fake_jira, pul
122120
108: [
123121
"set_cla_status",
124122
"initial_state",
125-
"synchronize_labels",
126123
"create_ospr_issue",
127124
"transition_jira_issue",
128125
"update_labels_on_pull_request",
@@ -132,7 +129,6 @@ def test_rescan_repository_dry_run(rescannable_repo, fake_github, fake_jira, pul
132129
110: [
133130
"set_cla_status",
134131
"initial_state",
135-
"synchronize_labels",
136132
"transition_jira_issue",
137133
"update_jira_issue",
138134
"update_labels_on_pull_request",

tests/test_synchronize_labels.py

Lines changed: 0 additions & 104 deletions
This file was deleted.

0 commit comments

Comments
 (0)