-
Notifications
You must be signed in to change notification settings - Fork 544
feat(github): Add repositories multiplexer #8074
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
feat(github): Add repositories multiplexer #8074
Conversation
This PR has the following changes to source plugin(s) tables:
|
| for _, w := range workflows.Workflows { | ||
| res <- Workflow{Workflow: w, Repository: *repo.Name} | ||
| } | ||
| opts.Page = resp.NextPage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a bug here, this should be actionOpts.Page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
| func buildRepositiories(t *testing.T, ctrl *gomock.Controller) client.GithubServices { | ||
| mock := mocks.NewMockRepositoriesService(ctrl) | ||
|
|
||
| var cs github.Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to GithubMockTestHelper as needed by the multiplexer
| orgRepos := make(map[string][]*github.Repository) | ||
| for _, org := range orgs { | ||
| for { | ||
| repos, resp, err := client.Repositories.ListByOrg(ctx, org, opts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're here.... it would be really nice if this also supported personal accounts, not just orgs 😄
But happy to leave that for a future update also
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think it's another feature, as our spec says orgs and we'll need to verify the API calls in all the resources (I think not all GitHub APIs are supported for personal accounts)
Co-authored-by: Herman Schaaf <[email protected]>
hermanschaaf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
🤖 I have created a release *beep* *boop* --- ## [4.0.0](plugins-source-github-v3.0.1...plugins-source-github-v4.0.0) (2023-02-21) ### ⚠ BREAKING CHANGES * **github:** Remove `repository` column from `github_workflows` * **github:** Change `github_organization_dependabot_alerts` PK from `(org,number)` to `(org,html_url)` ([#8070](#8070)) ### Features * **github-resources:** Add Traffic resources ([#8085](#8085)) ([cc22f56](cc22f56)) * **github:** Add repositories multiplexer ([#8074](#8074)) ([f5f874d](f5f874d)) * **github:** Handle secondary rate limit ([#8078](#8078)) ([bf1f1cc](bf1f1cc)) * **github:** Remove `repository` column from `github_workflows` ([21905a4](21905a4)) ### Bug Fixes * **deps:** Update module github.com/cloudquery/plugin-sdk to v1.38.2 ([#8156](#8156)) ([ac2d2d7](ac2d2d7)) * **deps:** Update module golang.org/x/net to v0.7.0 [SECURITY] ([#8176](#8176)) ([fc4cef8](fc4cef8)) * **github:** Add `repository_id` to `github_workflows` PK ([21905a4](21905a4)) * **github:** Change `github_organization_dependabot_alerts` PK from `(org,number)` to `(org,html_url)` ([#8070](#8070)) ([a7c64cd](a7c64cd)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
A first step for implementing #6247.
The APIs in that issue require passing a repository. We already list those twice for
github_workflowsandgithub_repositoriesso I think it's time to add a multiplexer