Use 'SetBaseURL' to correctly set GitLab URL based on user params. #229
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change updates server/server.go to set the GitLab URL using the `Client.SetBaseURL` to set the base URL off of the configured param. Previosuly even if a user set the `--gitlab-hostname` the default URL for GitLab would not overriden to match this. Closes hootsuite#227
|
|
lkysow
reviewed
Jan 15, 2018
server/server.go
Outdated
| @@ -105,6 +105,10 @@ func NewServer(config Config) (*Server, error) { | |||
| gitlabClient = &vcs.GitlabClient{ | |||
| Client: gitlab.NewClient(nil, config.GitlabToken), | |||
| } | |||
| err := gitlabClient.Client.SetBaseURL(config.GitlabHostname) | |||
There was a problem hiding this comment.
The default base URL is
defaultBaseURL = "https://gitlab.com/api/v4/"
based on the go-gitlab library (https://github.com/xanzy/go-gitlab/blob/c85e4e9d786c10e15e28446456f0e8918561b2d9/gitlab.go#L39)
We default to gitlab.com so if we're always going to call SetBaseURL then we'll need to convert it to the same format with https:// and /api/v4/.
Based on https://docs.gitlab.com/ce/api/#basic-usage I think we can assume that if you're running GitLab Enterprise that your API will be available at https://{hostname}/api/v4/. @jrasell is it possible for you to confirm that?
If so, then this PR needs to
- extract the protocol and hostname from the config.GitlabHostname string (in case the user adds the path we'll want to strip it)
- add the
/api/v4/path
Something like
supportedVCSHosts = append(supportedVCSHosts, vcs.Gitlab)
gitlabClient = &vcs.GitlabClient{
Client: gitlab.NewClient(nil, config.GitlabToken),
}
// We need to ensure the BaseURL has /api/v4/ appended. We first
// parse the URL because we only care about the Scheme and Host and
// want to discard anything else.
u, err := url.Parse(config.GitlabHostname)
if err != nil {
return nil, errors.Wrap(err, "parsing Gitlab hostname")
}
if err := gitlabClient.Client.SetBaseURL(fmt.Sprintf("%s://%s/api/v4/", u.Scheme, u.Host)); err != nil {
return nil, err
}
Closed
lkysow
reviewed
Jan 16, 2018
| } | ||
| if err := gitlabClient.Client.SetBaseURL(fmt.Sprintf("%s://%s/api/v4/", u.Scheme, u.Host)); err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
I pulled in your commits and got them working in #231.
I wanted to explain the changes I made and why. Here's what I ended up with:
// If not using gitlab.com we need to set the URL to the API.
if config.GitlabHostname != "gitlab.com" {
// Check if they've also provided a scheme so we don't prepend it
// again.
scheme := "https"
schemeSplit := strings.Split(config.GitlabHostname, "://")
if len(schemeSplit) > 1 {
scheme = schemeSplit[0]
config.GitlabHostname = schemeSplit[1]
}
apiURL := fmt.Sprintf("%s://%s/api/v4/", scheme, config.GitlabHostname)
if err := gitlabClient.Client.SetBaseURL(apiURL); err != nil {
return nil, errors.Wrapf(err, "setting GitLab API URL: %s", apiURL)
}
}
- I decided that if they hadn't specified a custom gitlab url that we shouldn't bother going through calling
SetBaseURLbecause it's more complicated as a reader of the code if I don't care about that branch - I didn't include the ticket number in the comment because as a reader of the code, I don't want to have to go to github to find out why something was done. Hopefully the comment explains everything.
- I ended up having to handle the case when the user prepends
https://to the option becauseurl.Parsedoesn't complain when we then create a url withhttps://https://gitlab.comso I needed to catch that sooner.
|
Commits pulled in in #231. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change updates server/server.go to set the GitLab URL using the
Client.SetBaseURLto set the base URL off of the configured param. Previously even if a user set the--gitlab-hostname, the default URL for GitLab would not overridden to match this.Tests run locally:
Closes #227