diff --git a/changelog.d/20230614_160046_kdmc_depr_people.rst b/changelog.d/20230614_160046_kdmc_depr_people.rst new file mode 100644 index 00000000..cd630768 --- /dev/null +++ b/changelog.d/20230614_160046_kdmc_depr_people.rst @@ -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 `_ tool. The ``label.yaml`` file is now unused and can be safely deleted from any openedx-webhooks data repositories. diff --git a/openedx_webhooks/info.py b/openedx_webhooks/info.py index fbf6f1ba..a6a3561f 100644 --- a/openedx_webhooks/info.py +++ b/openedx_webhooks/info.py @@ -121,6 +121,7 @@ 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()): @@ -128,9 +129,6 @@ def get_orgs_file(): 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 diff --git a/openedx_webhooks/jira_views.py b/openedx_webhooks/jira_views.py index 105049db..5a9bb067 100644 --- a/openedx_webhooks/jira_views.py +++ b/openedx_webhooks/jira_views.py @@ -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, @@ -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} diff --git a/openedx_webhooks/tasks/github_work.py b/openedx_webhooks/tasks/github_work.py index ed6bd81e..91daa08e 100644 --- a/openedx_webhooks/tasks/github_work.py +++ b/openedx_webhooks/tasks/github_work.py @@ -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]]: @@ -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) diff --git a/openedx_webhooks/tasks/pr_tracking.py b/openedx_webhooks/tasks/pr_tracking.py index dc0ff87c..b3e6a26f 100644 --- a/openedx_webhooks/tasks/pr_tracking.py +++ b/openedx_webhooks/tasks/pr_tracking.py @@ -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 @@ -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, diff --git a/tests/repo_data/openedx/openedx-webhooks-data/labels.yaml b/tests/repo_data/openedx/openedx-webhooks-data/labels.yaml deleted file mode 100644 index 0f32acb1..00000000 --- a/tests/repo_data/openedx/openedx-webhooks-data/labels.yaml +++ /dev/null @@ -1,11 +0,0 @@ -# Labels to be synchronized - -basic label: - color: bfe5bf -obsolete-label: - delete: true -important-label: - color: 00ff00 - description: This stuff is important. -not needed label: - delete: true diff --git a/tests/test_pull_request_opened.py b/tests/test_pull_request_opened.py index 7ed9d668..8e4e6517 100644 --- a/tests/test_pull_request_opened.py +++ b/tests/test_pull_request_opened.py @@ -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) @@ -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") @@ -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 @@ -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() @@ -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 @@ -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() @@ -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() @@ -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 @@ -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( @@ -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 @@ -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 @@ -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) diff --git a/tests/test_rescan.py b/tests/test_rescan.py index 20fd96ce..fed7f268 100644 --- a/tests/test_rescan.py +++ b/tests/test_rescan.py @@ -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", @@ -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", @@ -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", @@ -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", diff --git a/tests/test_synchronize_labels.py b/tests/test_synchronize_labels.py deleted file mode 100644 index 258ab967..00000000 --- a/tests/test_synchronize_labels.py +++ /dev/null @@ -1,104 +0,0 @@ -"""Tests of task/github.py:synchronize_labels.""" - -import pytest - -from openedx_webhooks.tasks.github_work import synchronize_labels - -from .fake_github import Label - - -# These tests should run when we want to test flaky GitHub behavior. -pytestmark = pytest.mark.flaky_github - - -DESIRED_LABELS = [ - Label(name='basic label', color='bfe5bf', description=None), - Label(name='important-label', color='00ff00', description='This stuff is important.'), - Label(name='something', color='123456', description='Huh?'), -] - -def test_no_labels_yet(fake_github): - repo = fake_github.make_repo("edx", "some-repo") - repo.set_labels([ - {"name": "something", "color": "123456", "description": "Huh?"}, - ]) - - synchronize_labels("edx/some-repo") - - assert repo.get_labels() == DESIRED_LABELS - assert len(fake_github.requests_made(method="POST")) == 2 - -def test_no_sync_needed(fake_github): - repo = fake_github.make_repo("edx", "some-repo") - repo.set_labels([ - {"name": "something", "color": "123456", "description": "Huh?"}, - {"name": "important-label", "color": "00ff00", "description": "This stuff is important."}, - {"name": "basic label", "color": "bfe5bf"}, - ]) - - synchronize_labels("edx/some-repo") - - assert repo.get_labels() == DESIRED_LABELS - assert fake_github.requests_made(method="POST") == [] - assert fake_github.requests_made(method="PATCH") == [] - assert fake_github.requests_made(method="DELETE") == [] - - -def test_one_is_missing(fake_github): - repo = fake_github.make_repo("edx", "some-repo") - repo.set_labels([ - {"name": "something", "color": "123456", "description": "Huh?"}, - {"name": "basic label", "color": "bfe5bf"}, - ]) - - synchronize_labels("edx/some-repo") - - assert repo.get_labels() == DESIRED_LABELS - assert len(fake_github.requests_made(method="POST")) == 1 - - -def test_color_is_wrong(fake_github): - repo = fake_github.make_repo("edx", "some-repo") - repo.set_labels([ - {"name": "something", "color": "123456", "description": "Huh?"}, - {"name": "important-label", "color": "ff0000", "description": "This stuff is important."}, - {"name": "basic label", "color": "bfe5bf"}, - ]) - - synchronize_labels("edx/some-repo") - - assert repo.get_labels() == DESIRED_LABELS - assert len(fake_github.requests_made(method="POST")) == 0 - assert len(fake_github.requests_made(method="PATCH")) == 1 - - -def test_description_is_wrong(fake_github): - repo = fake_github.make_repo("edx", "some-repo") - repo.set_labels([ - {"name": "something", "color": "123456", "description": "Huh?"}, - {"name": "important-label", "color": "00ff00", "description": "not sure"}, - {"name": "basic label", "color": "bfe5bf"}, - ]) - - synchronize_labels("edx/some-repo") - - assert repo.get_labels() == DESIRED_LABELS - assert len(fake_github.requests_made(method="POST")) == 0 - assert len(fake_github.requests_made(method="PATCH")) == 1 - - -def test_delete_unneeded(fake_github): - repo = fake_github.make_repo("edx", "some-repo") - repo.set_labels([ - {"name": "something", "color": "123456", "description": "Huh?"}, - {"name": "important-label", "color": "00ff00", "description": "not sure"}, - {"name": "basic label", "color": "bfe5bf"}, - {"name": "not needed label", "color": "ff0000"}, - ]) - - synchronize_labels("edx/some-repo") - - assert repo.get_labels() == DESIRED_LABELS - assert len(fake_github.requests_made(method="POST")) == 0 - assert len(fake_github.requests_made(method="PATCH")) == 1 - assert len(fake_github.requests_made(method="DELETE")) == 1