Add handling for github enterprise #114
Conversation
Signed-off-by: kbilchenko <[email protected]>
Signed-off-by: kbilchenko <[email protected]>
| @@ -91,6 +93,7 @@ func NewGitHubClient(source Source) (*GitHubClient, error) { | |||
| v4URL = source.GitHubAPIURL + "graphql" | |||
| } | |||
| clientV4 = githubv4.NewEnterpriseClient(v4URL, httpClient) | |||
| isEnterprise = true | |||
There was a problem hiding this comment.
this is basically saying once souce.GitHubAPIURL is provided then it is an enterprise github deployment. Is that true? Could a github deployment be public but not enterprise version?
There was a problem hiding this comment.
in this one case I belive if you are providing something what is not default github api, we should consider this as github enterprise but I may be wrong there
There was a problem hiding this comment.
at least isEnterprise = true should be set whenever NewEnterpriseClient is called
There was a problem hiding this comment.
this is what is happening, when we have case with custom url, we are assuming that enterprise client will be used
| ) | ||
|
|
||
| func (g *GitHubClient) listReleasesV4EnterPrice() ([]*github.RepositoryRelease, error) { |
There was a problem hiding this comment.
you already have g.isEnterprise available in GitHubClient, so you could modify that existing list releases method instead of adding an almost identicle one
There was a problem hiding this comment.
I know, but since it's a temporary solution, the logic of the method is almost the same, but for every reference I need to use another payload for making request, so I decided just to create new one, instead of huge if-else condition everywhere
|
@xtremerui can you check my answers, and let me know if it's enough for now? |
| @@ -91,6 +93,7 @@ func NewGitHubClient(source Source) (*GitHubClient, error) { | |||
| v4URL = source.GitHubAPIURL + "graphql" | |||
| } | |||
| clientV4 = githubv4.NewEnterpriseClient(v4URL, httpClient) | |||
| isEnterprise = true | |||
| } | |||
|
|
|||
| if source.GitHubV4APIURL != "" { | |||
There was a problem hiding this comment.
should isEnterprise be set here as well? Since here you are also creating a v4 client
…is set to custom Signed-off-by: kbilchenko <[email protected]>
Co-authored-by: Rui Yang <[email protected]> Signed-off-by: kbilchenko <[email protected]>
kirillbilchenko
force-pushed
the
github_enterprise_fix
branch
from
7774883
to
9128ad0
Compare
|
@xtremerui just a question, you merged this, but it was never released as a new version, any reason for this? |
|
@kirillbilchenko we don't do a release of resource type itself usually. It will be with concourse 7.7. |
Adding handling for github enterprise when the model looks different
Fixing issue with 109
Signed-off-by: kbilchenko [email protected]