Skip to content
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

Add graphql path when creating client #863

Merged
merged 4 commits into from
Sep 12, 2022

Conversation

gerardsegarra
Copy link
Contributor

@gerardsegarra gerardsegarra commented Sep 12, 2022

Fix 862

After investigating for a while, I found that the problem is that shurcooL/githubv4 expects a graphql url.

Just adding the graphql path to the URL parameter doesn't work (as that same url is used to pull the repo), but just by appending /api/graphql to the s.URL when initiating the client, fixes it.

Based on official documentation, the enterprise url for GraphQL is https://<HOST>/api/graphql.

Additional Information

Tradeoff

Potential improvement

Tests are missing for all the custom URLs.

@gerardsegarra gerardsegarra changed the title ADd graphql path when creating client Add graphql path when creating client Sep 12, 2022
Copy link
Member

@olblak olblak left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this.
I don't have access to a GitHub enterprise so my knowledge is purely theorical

Could I ask you to use the standard url library to do a clean url join
https://pkg.go.dev/net/url@go1.19beta1#JoinPath

something like


graphqlURL , err := url.JoinPath(s.URL, "/api/graphql")
if err != nil { 
    return nil, err
}
g.client = githubv4.NewEnterpriseClient(graphqlURL, httpClient)

@gerardsegarra
Copy link
Contributor Author

Done :D
Thank you for being so quick reviewing this.

@olblak
Copy link
Member

olblak commented Sep 12, 2022

Done :D Thank you for being so quick reviewing this.

Thanks for fixing this part, if all tests are green, I can cut a release

lemeurherve
lemeurherve previously approved these changes Sep 12, 2022
pkg/plugins/scms/github/main.go Outdated Show resolved Hide resolved
Co-authored-by: Hervé Le Meur <91831478+lemeurherve@users.noreply.github.com>
@olblak olblak added bug Something isn't working scm-github SCM of type GiHhub pullrequest-github labels Sep 12, 2022
@olblak olblak merged commit 4cd2e1c into updatecli:main Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pullrequest-github scm-github SCM of type GiHhub
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating pull request for GitHub custom URLs is not working.
4 participants