tools: adding slackbot for PR notifications by alyssawilk · Pull Request #16750 · envoyproxy/envoy

alyssawilk · 2021-06-01T16:11:00Z

We've been having PRs slipping through the cracks because repokitten assigns API reviewers and maintainer-on-call sees PRs as assigned and doesn't necessarily tag a maintainer for review.

Also adding hopefully helpful reminders for PRs which get stale. These will be less useful if folks don't use the waiting tag when waiting for response so will kick off a discussion thread on #maintainers about that

Risk Level: n/a
Testing: manual
Docs Changes: n/a
Release Notes: n/a
co-authored-by: mattklein123

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

repokitteh-read-only · 2021-06-01T16:11:07Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #16750 was opened by alyssawilk.

see: more, trace.

alyssawilk

huh, I thought I ran format_pre locally before I sent this out. looking

alyssawilk · 2021-06-01T16:21:06Z

tools/slack_tools/collect_prs.py

+
+        # If the PR hasn't been updated recently, don't nag.
+        # We can revisit this time, and always warn.
+        now = datetime.datetime.now()


If folks are likely to use this as their daily reminder, maybe we should warn for anything assigned? it is nice to have one place to look through

Actually I'm just going to update this one now.

Thing I don't understand, how the datetime returned by github doesn't come with a time zone - I did not expect to have to do the shifting manually but the timestamps were off unless I did. If someone with better python skills has ideas lmk

mattklein123 · 2021-06-01T16:24:28Z

tools/slack_tools/collect_prs.py

@@ -0,0 +1,192 @@
+# Script for collecting PRs in need of review, and informing maintainers via


It's up to you, but my preference is to just run this as a daily GH action cron. It looks like it should be pretty easy to use an action with code that exists in the repo: https://docs.github.com/en/actions/learn-github-actions/finding-and-customizing-actions?learn=getting_started#referencing-an-action-in-the-same-repository-where-a-workflow-file-uses-the-action.

Do you want to do this as a follow up or try to get this working now? I can help offline with credentials, etc.

I think follow-up if that's OK

Sure that's fine. Per offline convo I will help set up a GH action template that we can move this to in parallel.

phlax

hi @alyssawilk ive done a quick first review

its not clear to me how this code is triggered - im happy to review/test further once i understand better

tools/slack_tools/collect_prs.py

phlax · 2021-06-01T16:21:08Z

tools/slack_tools/collect_prs.py

+                waiting_prs.append("PR %s' is tagged as waiting" % (pr_title))
+                waiting = True
+                continue
+        if (waiting):


unnecessary parentheses

phlax · 2021-06-01T16:23:07Z

tools/slack_tools/collect_prs.py

+    stalled_prs = ""
+
+    # Snag all PRs, including drafts
+    for pr_info in repo.get_pulls("open"):


this loop is quite long and hard to follow - is it possible to break into some functions - it would make it more expressive and allow a load of dedenting

tools/slack_tools/collect_prs.py

phlax · 2021-06-01T16:24:06Z

tools/slack_tools/collect_prs.py

+
+
+from slack_sdk import WebClient
+from slack_sdk.errors import SlackApiError


these need to be at the top - but i guess ci has told you that already

phlax · 2021-06-01T16:24:50Z

tools/slack_tools/collect_prs.sh

@@ -0,0 +1,7 @@
+#!/bin/bash
+
+. tools/shell_utils.sh


im very much trying to get rid of this - can we use bazel for dependencies?

Do we have examples of scripts that run that way? my tooling skills are pretty weak so I just copied this from one of the other scrips

i can follow up after and shift this to bazel if thats helpful

Ultimately we are going to run this as a GH action. I think it will be much easier if we just stick with raw pip for this tool since I think it will be trivial to get running in GHA. So I think the end result here is we will lose the SH file and just keep the requirements.txt file. I will follow up once I do a template for GHA.

if we are using gh actions to trigger this we probs want to use something like this i think https://github.com/marketplace/actions/pip-installer

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

alyssawilk

ah, CI failure is dependabot. I'm going to fix in the next push becasue I'm not sure if the way phlax is suggesting I rework deps will change

alyssawilk · 2021-06-01T16:30:57Z

tools/slack_tools/collect_prs.sh

@@ -0,0 +1,7 @@
+#!/bin/bash
+
+. tools/shell_utils.sh


Do we have examples of scripts that run that way? my tooling skills are pretty weak so I just copied this from one of the other scrips

alyssawilk · 2021-06-01T16:31:17Z

tools/slack_tools/collect_prs.py

@@ -0,0 +1,192 @@
+# Script for collecting PRs in need of review, and informing maintainers via


I think follow-up if that's OK

phlax · 2021-06-01T16:36:33Z

tools/slack_tools/collect_prs.py

-    pass  # Python 3
-
-maintainers = {
+Maintainers = {


sorry to be annoying - the convention for constants in python is all caps - ie MAINTAINERS

haha, not annoying at all. I thought that request was kinda weird because I read it as Maintainers - makes much more sense w.r.t. python style now I'm not misreading it!

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mattklein123 · 2021-06-01T21:03:10Z

Cross referencing https://github.com/envoyproxy/envoy/pull/16756/files

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

alyssawilk · 2021-06-02T14:14:55Z

Any other requests for the scripts?

If not should I move them to .github/actions/pr_notifier/ ?

mattklein123 · 2021-06-02T14:59:07Z

@alyssawilk I would say let's move it and wire it up. We can launch and iterate. I will help you with the secret stuff offline.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mattklein123 · 2021-06-02T15:19:44Z

.github/workflows/pr_notifier.yml

@@ -0,0 +1,22 @@
+on:


I would recommend leaving the manual trigger option that I had in the example. This will allow you to manually trigger it from the UI to test it and also if for whatever reason the run fails and we want to run it.

mattklein123 · 2021-06-02T15:19:54Z

.github/workflows/pr_notifier.yml

+        run: |
+          python -m pip install --upgrade pip
+          pip install -r ./.github/actions/pr_notifier/requirements.txt
+      - name: Display Python version


nit: update to better text

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mattklein123 · 2021-06-02T15:20:25Z

.github/workflows/pr_notifier.yml

+        run: |
+          python -m pip install --upgrade pip
+          pip install -r ./.github/actions/pr_notifier/requirements.txt
+      - name: Display Python version


You will need to add the env var for the token secret here.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mattklein123

Awesome!!! I would just force merge it and you can test it manually.

phlax · 2021-06-02T15:38:46Z

@alyssawilk this still needs the dependabot setup i think

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

alyssawilk added 2 commits May 27, 2021 17:05

slackbot v1

2d7afe6

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

now with oncall notifications

10dda0d

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

alyssawilk assigned phlax and mattklein123 Jun 1, 2021

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jun 1, 2021

alyssawilk commented Jun 1, 2021

View reviewed changes

mattklein123 reviewed Jun 1, 2021

View reviewed changes

phlax requested changes Jun 1, 2021

View reviewed changes

comments v1

71e0d7b

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

alyssawilk commented Jun 1, 2021

View reviewed changes

phlax reviewed Jun 1, 2021

View reviewed changes

alyssawilk added 3 commits June 1, 2021 12:38

whoops!

49789cb

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

comments

57762ae

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mistype

88da62d

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

sorting!

f1b5d38

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mattklein123 added the waiting label Jun 2, 2021

merging

01e67e9

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

repokitteh-read-only bot removed the waiting label Jun 2, 2021

name

721d9db

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mattklein123 reviewed Jun 2, 2021

View reviewed changes

5am

fe68301

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mattklein123 reviewed Jun 2, 2021

View reviewed changes

mattklein123 added the waiting label Jun 2, 2021

comments

4cbedea

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

repokitteh-read-only bot removed the waiting label Jun 2, 2021

mattklein123 previously approved these changes Jun 2, 2021

View reviewed changes

phlax previously approved these changes Jun 2, 2021

View reviewed changes

deps

b292b52

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mattklein123 added the waiting label Jun 2, 2021

Fixing a bug with WIP

d57640e

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

alyssawilk dismissed stale reviews from phlax and mattklein123 via d57640e June 2, 2021 17:05

repokitteh-read-only bot removed the waiting label Jun 2, 2021

clarify

567089c

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mattklein123 previously approved these changes Jun 2, 2021

View reviewed changes

CI fixes

68c5cf8

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

alyssawilk dismissed mattklein123’s stale review via 68c5cf8 June 2, 2021 17:54

/ :-(

2f60551

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mattklein123 approved these changes Jun 2, 2021

View reviewed changes

mattklein123 merged commit 6a4b0d5 into envoyproxy:main Jun 2, 2021

leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021

tools: adding slackbot for PR notifications (envoyproxy#16750)

c481fed

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

alyssawilk deleted the slackbot branch August 4, 2022 00:57

		@@ -0,0 +1,192 @@
		# Script for collecting PRs in need of review, and informing maintainers via



		from slack_sdk import WebClient
		from slack_sdk.errors import SlackApiError

Conversation

alyssawilk commented Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Jun 1, 2021

Uh oh!

alyssawilk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phlax left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alyssawilk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattklein123 commented Jun 1, 2021

Uh oh!

alyssawilk commented Jun 2, 2021

Uh oh!

mattklein123 commented Jun 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

phlax commented Jun 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alyssawilk commented Jun 1, 2021 •

edited

Loading