Skip to content

Conversation

@erezrokah
Copy link
Member

Summary

A first step for implementing #6247.
The APIs in that issue require passing a repository. We already list those twice for github_workflows and github_repositories so I think it's time to add a multiplexer

@erezrokah erezrokah requested review from a team and hermanschaaf and removed request for a team February 14, 2023 15:25
@cq-bot cq-bot added the github label Feb 14, 2023
@github-actions
Copy link

This PR has the following changes to source plugin(s) tables:

  • Table github_workflows: column added with name repository_id and type Int

for _, w := range workflows.Workflows {
res <- Workflow{Workflow: w, Repository: *repo.Name}
}
opts.Page = resp.NextPage
Copy link
Member Author

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

Copy link
Member

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
Copy link
Member Author

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)
Copy link
Member

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

Copy link
Member Author

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)

@erezrokah erezrokah added the automerge Automatically merge once required checks pass label Feb 14, 2023
Copy link
Member

@hermanschaaf hermanschaaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@kodiakhq kodiakhq bot merged commit f5f874d into cloudquery:main Feb 14, 2023
@erezrokah erezrokah deleted the feat/add_repositories_multiplexer branch February 14, 2023 15:48
kodiakhq bot pushed a commit that referenced this pull request Feb 21, 2023
🤖 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).
@erezrokah erezrokah mentioned this pull request Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Automatically merge once required checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants