Add discussion metrics weekly slack message #673
Conversation
Codecov ReportAttention:
📢 Thoughts on this report? Let us know!. |
| SELECT | ||
| discussions.username as username, | ||
| COUNT(discussions.username) as num_comments, | ||
| FROM | ||
| \`open_source.github_events\` AS discussions | ||
| WHERE | ||
| discussions.type = 'discussion_comment' | ||
| AND timestamp_diff( | ||
| CURRENT_TIMESTAMP(), | ||
| discussions.created_at, | ||
| day | ||
| ) <= 7 | ||
| AND discussions.user_type != 'external' | ||
| AND discussions.user_type != 'bot' | ||
| GROUP BY discussions.username | ||
| ORDER BY num_comments DESC |
There was a problem hiding this comment.
Out of scope for this PR, but I wonder if there is some standard way of running a local in-memory instance of SQLite to query against, or something similar? Otherwise we end up with a ton of untested code: we validate that the exact text of our SQL code is what we expect, but not what that SQL code actually does (ie, does it get the exact rows in the table we want it to?).
I could imagine using in-memory SQLite as our backend, since this looks like bog-standard SQL with no BigQuery specific extensions in it.
Just a thought - not a blocker by any means, and definitely not a thing to do in this PR, but something to think about, since we've had a few cases of queries not returning the data set we expect them to.
There was a problem hiding this comment.
Yeah, that'd be nice to just to get some validation that it's valid sql. Adding to the backlog of things to do for eng-pipes:
#676
There was a problem hiding this comment.
but yes, the dev environment setup for BigQuery leaves much to be desired....
I don't think most people who have worked in eng-pipes have even hooked up their env to BigQuery
| @@ -45,67 +67,67 @@ describe('slackScores Tests', function () { | |||
| | null | - (0/0) | | |||
| | ospo | - (0/0) | | |||
There was a problem hiding this comment.
Does this mean we always report ospo, even when no metrics are returned? Or is that just how the test is configured?
There was a problem hiding this comment.
it's just how the test is configured, it uses the teams in test/product-owners.yml
| ]; | ||
| const discussionCommenters = [ | ||
| { | ||
| username: 'luke_skywalker', |
There was a problem hiding this comment.
Missing a golden opportunity to have one of the commenters be chewbacca:
Chewbacca, 3 hours ago:
--------
Raaaaa-ra-ra awwww raaaaa!
- Sent from my iPhone
| discussionCommenters, | ||
| }); | ||
| await sendDiscussionMetrics(); | ||
| // Columns with links in them may seem a bit off, because the links won't actually appear in slack |
There was a problem hiding this comment.
Is there any way to fix this? Or does it basically require running the code through a markdown parser?
There was a problem hiding this comment.
It will require us to run the code through a markdown parser, looks nasty in tests but it's well formed in slack!
hubertdeng123
deleted the
hubertdeng123/add-discussion-metrics-slack-message
branch
This adds a weekly discussion metrics slack message. It'll look something like this:
It'll show the top 5 most active discussions and 5 most active Sentaurs]
part of getsentry/team-ospo#165