Revert "Reapply "feat(review): Automatically call for review when a PR become…" (#45186)

Reverts #45126

Author: chouetz

.github/workflows/ask_review.yml

@@ -126,3 +126,21 @@ jobs:
           features: legacy-tasks
       - name: Check agent telemetry metric list
         run: dda inv -- -e github.agenttelemetry-list-change-ack-check --pr-id=${{ github.event.pull_request.number }}
+
+  ask-reviews:
+    if: github.triggering_actor != 'dd-devflow[bot]' && github.event.action == 'labeled' && github.event.label.name == 'ask-review'
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        with:
+          persist-credentials: false
+      - name: Install dda
+        uses: ./.github/actions/install-dda
+        with:
+          features: legacy-tasks
+      - name: Ask for code reviews
+        env:
+          SLACK_DATADOG_AGENT_BOT_TOKEN : ${{ secrets.SLACK_DATADOG_AGENT_BOT_TOKEN }}
+          PR_REQUESTED_TEAMS: ${{ toJSON(github.event.pull_request.requested_teams) }}
+        run: dda inv -- -e issue.ask-reviews -p ${{ github.event.pull_request.number }}

.github/workflows/label-analysis.yml

@@ -6,7 +6,7 @@
 
 from invoke.tasks import task
 
-from tasks.libs.ciproviders.github_api import GithubAPI, get_events_info
+from tasks.libs.ciproviders.github_api import GithubAPI, ask_review_actor
 from tasks.libs.owners.parsing import search_owners
 from tasks.libs.pipeline.notifications import (
     DEFAULT_SLACK_CHANNEL,
@@ -21,44 +21,39 @@ def ask_reviews(_, pr_id):
     if 'backport' in pr.title.casefold():
         print("This is a backport PR, we don't need to ask for reviews.")
         return
-    if any(label.name == 'no-review' for label in pr.get_labels()):
-        print("This PR has the no-review label, we don't need to ask for reviews.")
-        return
-    actor, team = get_events_info(pr)
-    if team:  # This is a review request event, only a single team is concerned
-        reviewers = [f"@datadog/{team}"]
-    else:
+    if any(label.name == 'ask-review' for label in pr.get_labels()):
+        actor = ask_review_actor(pr)
         reviewers = [f"@datadog/{team['slug']}" for team in json.loads(os.environ['PR_REQUESTED_TEAMS'])]
-    print(f"Reviewers: {reviewers}")
-
-    from slack_sdk import WebClient
-
-    client = WebClient(os.environ['SLACK_DATADOG_AGENT_BOT_TOKEN'])
-    emojis = client.emoji_list()
-    waves = [emoji for emoji in emojis.data['emoji'] if 'wave' in emoji and 'microwave' not in emoji]
-
-    channels = defaultdict(list)
-    for reviewer in reviewers:
-        channel = next(
-            (chan for team, chan in GITHUB_SLACK_REVIEW_MAP.items() if team.casefold() == reviewer.casefold()),
-            DEFAULT_SLACK_CHANNEL,
-        )
-        channels[channel].append(reviewer)
-
-    for channel, reviewers in channels.items():
-        stop_updating = ""
-        if (pr.user.login == "renovate[bot]" or pr.user.login == "mend[bot]") and pr.title.startswith(
-            "chore(deps): update integrations-core"
-        ):
-            stop_updating = "Add the `stop-updating` label before trying to merge this PR, to prevent it from being updated by Renovate.\n"
-        message = f'Hello :{random.choice(waves)}:!\n*{actor}* is asking review for PR <{pr.html_url}/s|{pr.title}>.\nCould you please have a look?\n{stop_updating}Thanks in advance!\n'
-        if channel == DEFAULT_SLACK_CHANNEL:
-            message = f'Hello :{random.choice(waves)}:!\nA review channel is missing for {', '.join(reviewers)}, can you please ask them to update `github_slack_review_map.yaml` and transfer them this review <{pr.html_url}/s|{pr.title}>?\n Thanks in advance!'
-        try:
-            client.chat_postMessage(channel=channel, text=message)
-        except Exception as e:
-            message = f"An error occurred while sending a review message from {actor} for PR <{pr.html_url}/s|{pr.title}> to channel {channel}. Error: {e}"
-            client.chat_postMessage(channel=DEFAULT_SLACK_CHANNEL, text=message)
+        print(f"Reviewers: {reviewers}")
+
+        from slack_sdk import WebClient
+
+        client = WebClient(os.environ['SLACK_DATADOG_AGENT_BOT_TOKEN'])
+        emojis = client.emoji_list()
+        waves = [emoji for emoji in emojis.data['emoji'] if 'wave' in emoji and 'microwave' not in emoji]
+
+        channels = defaultdict(list)
+        for reviewer in reviewers:
+            channel = next(
+                (chan for team, chan in GITHUB_SLACK_REVIEW_MAP.items() if team.casefold() == reviewer.casefold()),
+                DEFAULT_SLACK_CHANNEL,
+            )
+            channels[channel].append(reviewer)
+
+        for channel, reviewers in channels.items():
+            stop_updating = ""
+            if (pr.user.login == "renovate[bot]" or pr.user.login == "mend[bot]") and pr.title.startswith(
+                "chore(deps): update integrations-core"
+            ):
+                stop_updating = "Add the `stop-updating` label before trying to merge this PR, to prevent it from being updated by Renovate.\n"
+            message = f'Hello :{random.choice(waves)}:!\n*{actor}* is asking review for PR <{pr.html_url}/s|{pr.title}>.\nCould you please have a look?\n{stop_updating}Thanks in advance!\n'
+            if channel == DEFAULT_SLACK_CHANNEL:
+                message = f'Hello :{random.choice(waves)}:!\nA review channel is missing for {', '.join(reviewers)}, can you please ask them to update `github_slack_review_map.yaml` and transfer them this review <{pr.html_url}/s|{pr.title}>?\n Thanks in advance!'
+            try:
+                client.chat_postMessage(channel=channel, text=message)
+            except Exception as e:
+                message = f"An error occurred while sending a review message from {actor} for PR <{pr.html_url}/s|{pr.title}> to channel {channel}. Error: {e}"
+                client.chat_postMessage(channel=DEFAULT_SLACK_CHANNEL, text=message)
 
 
 @task

tasks/issue.py

@@ -754,18 +754,10 @@ def create_release_pr(title, base_branch, target_branch, version, changelog_pr=F
     return create_datadog_agent_pr(title, base_branch, target_branch, milestone_name, labels)
 
 
-def get_events_info(pr):
-    actor, team = None, None
-    for event in reversed(list(pr.get_issue_events())):
-        if (event.event == "labeled" and event.label.name == "ask-review") or event.event in [
-            "review_requested",
-            "ready_for_review",
-        ]:
-            actor = event.actor.name or event.actor.login
-            if "requested_team" in event.raw_data:
-                team = event.raw_data["requested_team"]["slug"]
-            break
-    return actor, team
+def ask_review_actor(pr):
+    for event in pr.get_issue_events():
+        if event.event == "labeled" and event.label.name == "ask-review":
+            return event.actor.name or event.actor.login
 
 
 def generate_local_github_token(ctx):

tasks/libs/ciproviders/github_api.py

@@ -143,10 +143,10 @@ class TestAskReviews(unittest.TestCase):
         'os.environ',
         {'PR_REQUESTED_TEAMS': '[{"slug": "team1"}, {"slug": "team2"}]', 'SLACK_DATADOG_AGENT_BOT_TOKEN': 'fake-token'},
     )
-    @patch('tasks.issue.get_events_info')
+    @patch('tasks.issue.ask_review_actor')
     @patch('tasks.issue.GithubAPI')
     @patch('slack_sdk.WebClient')
-    def test_ask_reviews_nominal(self, slack_mock, gh_mock, get_events_info_mock, print_mock):
+    def test_ask_reviews_nominal(self, slack_mock, gh_mock, ask_review_actor_mock, print_mock):
         pr_mock = MagicMock()
         pr_mock.title = "This is a feature"
         pr_mock.get_labels.return_value = [types.SimpleNamespace(name='ask-review')]
@@ -157,7 +157,7 @@ def test_ask_reviews_nominal(self, slack_mock, gh_mock, get_events_info_mock, pr
         gh_instance = MagicMock()
         gh_instance.repo.get_pull.return_value = pr_mock
         gh_mock.return_value = gh_instance
-        get_events_info_mock.return_value = ("actor", None)
+        ask_review_actor_mock.return_value = "actor"
 
         emoji_list = {'emoji': {'wave': 'url1', 'waves': 'url2', 'microwave': 'urlx'}}
         slack_client = MagicMock()
@@ -179,17 +179,17 @@ def test_ask_reviews_nominal(self, slack_mock, gh_mock, get_events_info_mock, pr
         'os.environ',
         {'PR_REQUESTED_TEAMS': '[{"slug": "team1"}, {"slug": "team2"}]', 'SLACK_DATADOG_AGENT_BOT_TOKEN': 'fake-token'},
     )
-    @patch('tasks.issue.get_events_info')
+    @patch('tasks.issue.ask_review_actor')
     @patch('tasks.issue.GithubAPI')
-    def test_ask_reviews_backport(self, gh_mock, get_events_info_mock, print_mock):
+    def test_ask_reviews_backport(self, gh_mock, ask_review_actor_mock, print_mock):
         pr_mock = MagicMock()
         pr_mock.title = "Backport: fix issue 123"
         pr_mock.get_labels.return_value = [MagicMock(name='ask-review')]
         gh_instance = MagicMock()
         gh_instance.repo.get_pull.return_value = pr_mock
         gh_mock.return_value = gh_instance
 
-        get_events_info_mock.return_value = ("actor", None)
+        ask_review_actor_mock.return_value = "actor"
 
         ask_reviews(MockContext(), 6)
         print_mock.assert_any_call("This is a backport PR, we don't need to ask for reviews.")
@@ -202,10 +202,10 @@ def test_ask_reviews_backport(self, gh_mock, get_events_info_mock, print_mock):
             'SLACK_DATADOG_AGENT_BOT_TOKEN': 'fake-token',
         },
     )
-    @patch('tasks.issue.get_events_info')
+    @patch('tasks.issue.ask_review_actor')
     @patch('tasks.issue.GithubAPI')
     @patch('slack_sdk.WebClient')
-    def test_ask_reviews_default_channel(self, slack_mock, gh_mock, get_events_info_mock, print_mock):
+    def test_ask_reviews_default_channel(self, slack_mock, gh_mock, ask_review_actor_mock, print_mock):
         pr_mock = MagicMock()
         pr_mock.title = "Title"
         pr_mock.get_labels.return_value = [types.SimpleNamespace(name='ask-review')]
@@ -215,7 +215,7 @@ def test_ask_reviews_default_channel(self, slack_mock, gh_mock, get_events_info_
         gh_instance = MagicMock()
         gh_instance.repo.get_pull.return_value = pr_mock
         gh_mock.return_value = gh_instance
-        get_events_info_mock.return_value = ("actor", None)
+        ask_review_actor_mock.return_value = "actor"
 
         emoji_list = {'emoji': {'wave': 'url1'}}
         slack_client = MagicMock()
@@ -239,10 +239,10 @@ def test_ask_reviews_default_channel(self, slack_mock, gh_mock, get_events_info_
         'os.environ',
         {'PR_REQUESTED_TEAMS': '[{"slug": "teamx"}, {"slug": "teamy"}]', 'SLACK_DATADOG_AGENT_BOT_TOKEN': 'fake-token'},
     )
-    @patch('tasks.issue.get_events_info')
+    @patch('tasks.issue.ask_review_actor')
     @patch('tasks.issue.GithubAPI')
     @patch('slack_sdk.WebClient')
-    def test_ask_reviews_same_slack_channel(self, slack_mock, gh_mock, get_events_info_mock, print_mock):
+    def test_ask_reviews_same_slack_channel(self, slack_mock, gh_mock, ask_review_actor_mock, print_mock):
         pr_mock = MagicMock()
         pr_mock.title = "Some PR"
         pr_mock.get_labels.return_value = [types.SimpleNamespace(name='ask-review')]
@@ -252,7 +252,7 @@ def test_ask_reviews_same_slack_channel(self, slack_mock, gh_mock, get_events_in
         gh_instance = MagicMock()
         gh_instance.repo.get_pull.return_value = pr_mock
         gh_mock.return_value = gh_instance
-        get_events_info_mock.return_value = ("actor", None)
+        ask_review_actor_mock.return_value = "actor"
 
         emoji_list = {'emoji': {'wave': 'url1', 'waves': 'url2'}}
         slack_client = MagicMock()
@@ -269,103 +269,3 @@ def test_ask_reviews_same_slack_channel(self, slack_mock, gh_mock, get_events_in
         slack_client.chat_postMessage.assert_called_once()
         args, kwargs = slack_client.chat_postMessage.call_args
         self.assertEqual(kwargs['channel'], 'chan-shared')
-
-    @patch('builtins.print')
-    @patch(
-        'os.environ',
-        {'PR_REQUESTED_TEAMS': '[{"slug": "team1"}, {"slug": "team2"}]', 'SLACK_DATADOG_AGENT_BOT_TOKEN': 'fake-token'},
-    )
-    @patch('tasks.issue.get_events_info')
-    @patch('tasks.issue.GithubAPI')
-    @patch('slack_sdk.WebClient')
-    def test_ask_reviews_from_review_request(self, slack_mock, gh_mock, get_events_info_mock, print_mock):
-        """Test that when a specific team is requested (review_request event), only that team is notified"""
-        pr_mock = MagicMock()
-        pr_mock.title = "Feature PR"
-        pr_mock.get_labels.return_value = [types.SimpleNamespace(name='ask-review')]
-        pr_mock.user.login = "someuser"
-        pr_mock.html_url = "https://github.com/foo/bar/pull/9"
-
-        gh_instance = MagicMock()
-        gh_instance.repo.get_pull.return_value = pr_mock
-        gh_mock.return_value = gh_instance
-        # Simulate a review_request event where team1 was specifically requested
-        get_events_info_mock.return_value = ("actor", "team42")
-
-        emoji_list = {'emoji': {'wave': 'url1', 'waves': 'url2'}}
-        slack_client = MagicMock()
-        slack_client.emoji_list.return_value = types.SimpleNamespace(data=emoji_list)
-        slack_mock.return_value = slack_client
-
-        # Fill GITHUB_SLACK_REVIEW_MAP
-        GITHUB_SLACK_REVIEW_MAP['@datadog/team42'] = 'channel42'
-        GITHUB_SLACK_REVIEW_MAP['@datadog/team2'] = 'channel2'
-
-        ask_reviews(MockContext(), 9)
-
-        # Only one message should be sent (to team1's channel)
-        slack_client.chat_postMessage.assert_called_once()
-        args, kwargs = slack_client.chat_postMessage.call_args
-        self.assertEqual(kwargs['channel'], 'channel42')
-
-    @patch('builtins.print')
-    @patch(
-        'os.environ',
-        {'PR_REQUESTED_TEAMS': '[{"slug": "team1"}, {"slug": "team2"}]', 'SLACK_DATADOG_AGENT_BOT_TOKEN': 'fake-token'},
-    )
-    @patch('tasks.issue.get_events_info')
-    @patch('tasks.issue.GithubAPI')
-    @patch('slack_sdk.WebClient')
-    def test_ask_reviews_from_ready_for_review(self, slack_mock, gh_mock, get_events_info_mock, print_mock):
-        """Test that when PR is marked as ready_for_review, all requested teams are notified"""
-        pr_mock = MagicMock()
-        pr_mock.title = "Draft PR now ready"
-        pr_mock.get_labels.return_value = [types.SimpleNamespace(name='ask-review')]
-        pr_mock.user.login = "developer"
-        pr_mock.html_url = "https://github.com/foo/bar/pull/10"
-
-        gh_instance = MagicMock()
-        gh_instance.repo.get_pull.return_value = pr_mock
-        gh_mock.return_value = gh_instance
-        # Simulate a ready_for_review event (team is None)
-        get_events_info_mock.return_value = ("actor", None)
-
-        emoji_list = {'emoji': {'wave': 'url1', 'waves': 'url2'}}
-        slack_client = MagicMock()
-        slack_client.emoji_list.return_value = types.SimpleNamespace(data=emoji_list)
-        slack_mock.return_value = slack_client
-
-        # Fill GITHUB_SLACK_REVIEW_MAP
-        GITHUB_SLACK_REVIEW_MAP['@datadog/team1'] = 'channel1'
-        GITHUB_SLACK_REVIEW_MAP['@datadog/team2'] = 'channel2'
-
-        ask_reviews(MockContext(), 10)
-
-        # Both teams should be notified
-        self.assertEqual(len(slack_client.chat_postMessage.mock_calls), 2)
-        channels = [call.kwargs['channel'] for call in slack_client.chat_postMessage.mock_calls]
-        self.assertIn('channel1', channels)
-        self.assertIn('channel2', channels)
-
-    @patch('builtins.print')
-    @patch(
-        'os.environ',
-        {'PR_REQUESTED_TEAMS': '[{"slug": "team1"}]', 'SLACK_DATADOG_AGENT_BOT_TOKEN': 'fake-token'},
-    )
-    @patch('tasks.issue.GithubAPI')
-    def test_ask_reviews_with_no_review_label(self, gh_mock, print_mock):
-        """Test that PR with no-review label is skipped"""
-        pr_mock = MagicMock()
-        pr_mock.title = "WIP: Experimental changes"
-        pr_mock.get_labels.return_value = [
-            types.SimpleNamespace(name='ask-review'),
-            types.SimpleNamespace(name='no-review'),
-        ]
-
-        gh_instance = MagicMock()
-        gh_instance.repo.get_pull.return_value = pr_mock
-        gh_mock.return_value = gh_instance
-
-        ask_reviews(MockContext(), 11)
-
-        print_mock.assert_any_call("This PR has the no-review label, we don't need to ask for reviews.")