Support GitHub Enterprise #64
Conversation
cmd/server/main.go
Outdated
| @@ -48,6 +49,10 @@ import ( | |||
| ) | |||
|
|
|||
| var ( | |||
| // custom GitHub API URLs | |||
| githubAPIRawURL = flag.String("github-api-base-url", "", "base URL for GitHub API. Please set this when you use GitHub Enterprise. This often is your GitHub Enterprise hostname. If the base URL does not have the suffix \"/api/v3/\", it will be added automatically.") | |||
| githubAPIRawUploadURL = flag.String("github-api-upload-url", "", "upload URL for GitHub API. This must be set when you set --github-api-base-url. This often is your GitHub Enterprise hostname. If the upload URL does not have the suffix \"/api/uploads\", it will be added automatically.") | |||
There was a problem hiding this comment.
Can this flag be removed?
- Triage Party should not perform uploads - it's read-only.
- As far as I know, the upload URL is usually the same as the base URL.
There was a problem hiding this comment.
Oh, that's good news. I will remove it.
As far as I know, the upload URL is usually the same as the base URL.
actually, it is slightly different.
baseURL:https://my.github.enterprise/api/v3/uploadURL:https://my.github.enterprise/api/uploads/
But, it's not a problem anyway.
cmd/server/main.go
Outdated
| ctx := context.Background() | ||
|
|
||
| client := github.NewClient(oauth2.NewClient(ctx, oauth2.StaticTokenSource( | ||
| &oauth2.Token{AccessToken: triage.MustReadToken(*githubTokenFile, "GITHUB_TOKEN")}, | ||
| ))) | ||
|
|
||
| // set GitHub API endpoints to client if custom endpoint was set. |
There was a problem hiding this comment.
The same logic should also be applied to cmd/tester/main.go.
Do you mind moving this code to pkg/triage/github.go? This is where I've been sneaking the logic that is shared between the two programs.
There was a problem hiding this comment.
Thanks for the catch. I will do it.
|
I resolved your review comments and squashed. Please take a look! |
pkg/triage/github.go
Outdated
| @@ -45,3 +47,11 @@ func MustReadToken(path string, env string) string { | |||
| } | |||
| return token | |||
| } | |||
|
|
|||
| func MustCreateGithubClient(githubAPIRawURL string, httpClient *http.Client) (*github.Client, error) { | |||
There was a problem hiding this comment.
This can be named just GitHubClient: Must as a prefix is reserved in go for functions which panic or otherwise exit on failure.
There was a problem hiding this comment.
I'd like to keep the name and change the method to exit when error.
pkg/triage/github.go
Outdated
|
|
||
| func MustCreateGithubClient(githubAPIRawURL string, httpClient *http.Client) (*github.Client, error) { | ||
| if githubAPIRawURL != "" { | ||
| // passing base url to uplaod URL is safe because triage-party does NOT upload anything. |
There was a problem hiding this comment.
the comment isn't really necessary, but if you want to keep it, fix the spelling of "upload". Thanks!
There was a problem hiding this comment.
OK. I'll remove it.
|
I fixed |
|
Ok
…On Thu, May 7, 2020, 4:08 AM Shingo Omura ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pkg/triage/github.go
<#64 (comment)>:
> @@ -45,3 +47,11 @@ func MustReadToken(path string, env string) string {
}
return token
}
+
+func MustCreateGithubClient(githubAPIRawURL string, httpClient *http.Client) (*github.Client, error) {
I'd like to keep the name and change the method to exit when error.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#64 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAYYMFKOO4WF6JBUQLSPKDRQKJCJANCNFSM4M2U5VSA>
.
|
|
Thank you! I am so glad that this feature landed before v1.0. |
Fixes #63
The PR enables just to configure GitHub API endpoints.
I didn't use
github.NewEnterpriseClient()method because this method does have a bug (google/go-github#1510) in upload URL completion which has been fixed only on the master branch.Although I'm not sure how to test my change (I mean CI actually), I confirmed this PR works for my own GitHub Enterprise manually.