-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expand to codecov
somehow
#482
Comments
Went around the horn in Slack about whether to have multiple apps or a single app that we would keep unlisted and filter out any unwanted org traffic from (if anyone were to bother to get it installed outside of our orgs). We decided to proceed with two apps for now to avoid having to come up with a brand that works for the general case vs. getsantry and CoveCod. |
Below is an audit of Line 11 in 94396e5
|
Thinking next about
|
I guess what we need is to adopt a strategy for ignoring codepaths that aren't relevant in |
Looking good! 👍
|
I guess as long as we don't subscribe CoveCod to |
And honestly it's really only the |
I guess the risk is that we receive an event from the |
|
Need to tackle these ones:
|
Yeah okay upgrading pubsub is going to be the main piece of work before we can start testing out sending CoveCod events to eng-pipes. |
And it won't impact Codecov to install CoveCod without updating pubsub, that should just continue to poke |
I put up a slew of incremental PRs. I need to understand pubsub better, here's the jobs in GCP. Since repos are coming in the payload I guess we should look at sending org in the payload as well, that means we have to set up one pubsub in GCP for each org vs. wiring org knowledge into eng-pipes. Tradeoffs. Wiring into org I guess would block on multi-org config changes. |
Looking into |
P.S. It shouldn't matter that we have logic for Status labels in eng-pipes, since those labels won't exist in We should set up a eng-pipes/src/utils/getOssUserType.ts Lines 63 to 81 in 6c1342f
|
Any additional bots we want to exclude? eng-pipes/src/utils/isFromABot.ts Lines 1 to 7 in 6c1342f
|
Alright I still don't know how pubsub works. Where do we define the payload? |
Also how are Slack notifications going to work? What else do we need to configure for that? |
(h/t) |
Agreed here. |
I think I found it! Pretty sure this is where we define prod secrets: https://github.com/getsentry/eng-pipes/settings/environments/164955041/edit |
There shouldn't be any coming from the bot since the eng-pipes/src/brain/issueNotifier/index.ts Lines 38 to 43 in 6b6a0ec
This isn't the end of the world since it requires manually sending to support. I'm pretty sure Support and Product Owner notifications should get folded together in Q3 under getsentry/team-ospo#157.
We're in the same boat here as with SDK repos where we don't provide Slack notifications currently. Let's solve this under getsentry/team-ospo#157 as well. |
If I'm reading this right we essentially skip notifications for repos without routing, which is what eng-pipes/src/webhooks/pubsub/slackNotifications.ts Lines 421 to 423 in 6b6a0ec
I think we're fine. |
If I'm wrong I expect an error to show up in Sentry. I'm going to set |
I see this in the console:
but nothing in Sentry. |
Hrm, possible this is broken in prod, too. 🤔
|
For getsentry/team-ospo#79 I need to roll out Sentry's routing and triage process in the
codecov
org. I've set up a second GitHub App for this purpose (yay CoveCod! :), and I need to send the GH event stream from that app somewhere for processing. If we can, it seems best to send both event streams (getsentry
andcodecov
) to our existingeng-pipes
deployment, so that we don't have to maintain two separate deployments. We could maintain two deployments, but we already have a third viable org (syntaxfm
; no immediate plans to roll out automations there) and should only expect more in the future as Sentry grows. I want a consistent GH engagement management process across all orgs. I would like to send everything through our singleeng-pipes
.The primary change here will be in
getClient
(I'm expecting some changes in the GitHub brainlets as well), and the biggest thing to talk through up front (assuming we're good with running everything through a single deployment) is configuration. We currently assume a single GitHub App, and we use these envvar keys to configure it:eng-pipes/.env.example
Lines 5 to 12 in af1f4ca
Supporting an arbitrary number of GitHub Apps (one per org) is a little futzy using envvars, but I think something like this might work?
I would parse this out at startup-time (somewhere in/under
buildServer.ts
), but only fail at runtime (as we do today) when there is no app configured for the requested org.Update
I ended up going with a
github-orgs.yml
for configuration, because a) I need to scope the repo allow-lists to orgs and those are too cumbersome for environment variables, and b) we want to move towards a YAML config file anyway to bring in some config fromsecurity-as-code
.PRs
The text was updated successfully, but these errors were encountered: