Skip to content
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

Lightly refactor main pubsub handler #513

Merged
merged 2 commits into from
Jul 10, 2023
Merged

Conversation

chadwhitacre
Copy link
Member

@chadwhitacre chadwhitacre commented Jul 8, 2023

Part of #482, after #512.

I'm pretty sure what I want to do is loop over multiple orgs in this handler (getsentry, codecov). This sets me up for that.

This also switches to requiring that repos be in payload. Can we do that? 🤔 Decided against this for now, too controversial.

@chadwhitacre chadwhitacre mentioned this pull request Jul 8, 2023
43 tasks
@chadwhitacre chadwhitacre force-pushed the cwlw/lightly-refactor-pubsub branch 3 times, most recently from b00d339 to f75a0fb Compare July 8, 2023 12:34
@chadwhitacre chadwhitacre force-pushed the cwlw/owner-to-org branch 2 times, most recently from 57a831c to 1fd3e8b Compare July 10, 2023 19:17
Base automatically changed from cwlw/owner-to-org to main July 10, 2023 19:35
src/webhooks/pubsub/index.ts Dismissed Show resolved Hide resolved
@chadwhitacre chadwhitacre merged commit 67b2ccf into main Jul 10, 2023
5 checks passed
@chadwhitacre chadwhitacre deleted the cwlw/lightly-refactor-pubsub branch July 10, 2023 19:49
@hubertdeng123
Copy link
Member

This also switches to requiring that repos be in payload. Can we do that? 🤔 Decided against this for now, too controversial.

What do you mean here? Isn't repos already in the payload?

@hubertdeng123
Copy link
Member

hubertdeng123 commented Jul 10, 2023

Do you mean hard requiring repos to be in the payload? And raising if it isn't? I guess we can do that but that's defined in the ops repo already so we have full control over that. https://github.com/getsentry/ops/blob/master/terraform/super-big-data/super-big-consumers/schedule.tf

@chadwhitacre
Copy link
Member Author

What do you mean here? Isn't repos already in the payload?

It's optional, though ...

repos?: string[];

... and we fall back to a default.

repos = payload.repos || DEFAULT_REPOS;

Where is the payload defined? In GCP somewhere?

@chadwhitacre
Copy link
Member Author

that's defined in the ops repo already so we have full control over that. https://github.com/getsentry/ops/blob/master/terraform/super-big-data/super-big-consumers/schedule.tf

The missing link! Thank you! 😁 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants