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

Breaks if pagination is activated #1

Closed
kenkendk opened this issue Mar 27, 2020 · 2 comments
Closed

Breaks if pagination is activated #1

kenkendk opened this issue Mar 27, 2020 · 2 comments

Comments

@kenkendk
Copy link

Since the Github API returns full paths for the next links, these cause invalid urls to be generated if the repository has many items.

You can test with:

> github-release info --user duplicati --repo duplicati
error: could not fetch tags, Get https://api.github.comhttps//api.github.com/repositories/22636263/tags?per_page=100&page=2: dial tcp: lookup api.github.comhttps: no such host

I have filed a proposed fix for the rest package here:
kevinburke/rest#8

The full link url is returned here:
https://github.com/meterup/github-release/blob/master/github/github.go#L244

Then the REST client is created here:
https://github.com/meterup/github-release/blob/master/github/github.go#L249

But this fails as the client has a base URL and does string concatenation:
https://github.com/kevinburke/rest/blob/master/client.go#L131

It could also be fixed in github.go by stripping the link and only passing the path part to the REST client, similar to this:

if strings.HasPrefix(nextLinkURL, c.client.Base) {
	nextLinkURL = nextLinkURL[len(c.client.Base):]
}

This also works, but is less robust, as Github could send out pagination links to another domain at some point. Not sure it you would rather wait for the rest package to be fixed or include a workaround in your package.

@kevinburke
Copy link

Thanks, I can reproduce!

kevinburke added a commit to kevinburke/rest that referenced this issue Mar 29, 2020
If you call NewRequest() with a path that matches the
set Base, we'll strip it from the path before making
the request. The alternative led to URL's like GET
https://api.github.com/https://api.github.com/foo/bar which is not
very helpful at all.

Fixes meterup/github-release#1.
Fixes #8.
kevinburke1 added a commit that referenced this issue Mar 29, 2020
Since #1 there is at least some subset of dependencies that will not
work with all clients; since this is a binary, it's not difficult to
vendor the correct set of dependencies, so do so.
@kevinburke1
Copy link

This should be fixed now, please let me know if you're still running into issues.

regisb added a commit to overhangio/tutor that referenced this issue Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants