git-machete icon indicating copy to clipboard operation
git-machete copied to clipboard

Temporary Redirect when creating a new pull request

Open tomaszferens opened this issue 2 years ago • 5 comments

Overview

Running git machete github create-pr results in an error message: GitHub API returned 307 HTTP status with error message: Temporary Redirect

Expected behavior/command output

Ability to create a PR using git-machete

Actual behavior/command output

Http request to Github API fails which means that PR is not being made

git and git machete versions

git --version
git version 2.32.1 (Apple Git-133)

git machete --version
git machete version 3.11.3

Additional diagnostics info

Creating a PR from 209/plugins to develop...__fire_github_api_request(method=POST, path=/repos/<org_redacted_by_me>/<repo_redacted_by_me>/pulls, token=***, request_body={'head': '209/plugins', 'base': 'develop', 'title': 'feat(web): #209 copy lexical plugins', 'body': '', 'draft': False}): firing a POST request to https://api.github.com/repos/<org_redacted_by_me>/<repo_redacted_by_me>/pulls with a bearer token and request body {"head": "209/plugins", "base": "develop", "title": "feat(web): #209 copy lexical plugins", "body": "", "draft": false}

GitHub API returned 307 HTTP status with error message: Temporary Redirect

Please open an issue regarding this topic under link: https://github.com/VirtusLab/git-machete/issues/new

tomaszferens avatar Jul 25 '22 16:07 tomaszferens

Thanks for reporting! Is my suspicion correct that the org/repo that you're firing the request against has been moved from a different org and/or renamed?

Anyway, we'll include the fix in the subsequent v3.11.4 release.

Btw if I may ask for "marketing" purposes: did you learn of this project via Benjamin Congdon's blog post?

PawelLipski avatar Jul 25 '22 17:07 PawelLipski

@PawelLipski correct we renamed the repository couple months ago.

Exactly! I found git machete because of this tweet: https://twitter.com/andrestaltz/status/1551277623594127361?s=21&t=_QoVRH5epu92_yYPOBVtDQ which essentially links to the blog post you mentioned.

Thanks!

tomaszferens avatar Jul 25 '22 19:07 tomaszferens

For the implementer: see urllib.request.HTTPRedirectHandler.redirect_request:

        if (not (code in (301, 302, 303, 307) and m in ("GET", "HEAD")
            or code in (301, 302, 303) and m == "POST")):
            raise HTTPError(req.full_url, code, msg, headers, fp)

Seems that 307 redirect on POST is treated as an error by urllib... I can't see the reason for that TBH (see https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/307).

A workaround could be to either handle 307 "error" explicitly in git_machete.github.__fire_github_api_request, or to override the default urllib.request.HTTPRedirectHandler with a custom class.

PawelLipski avatar Jul 26 '22 17:07 PawelLipski

@PawelLipski Maybe we should just send a post request to the "correct" (the renamed one) URL?

So, instead of sending request to: https://api.github.com/repos/<organization>/<old_repository_name>/pull we should send it to: https://api.github.com/repos/<organization>/<new_repository_name>/pull

Alternatively, looks like urllib.request is not following 307 Redirects due to the spec: https://stackoverflow.com/a/62385184/6372449

So, maybe we should get new url from Location headers and do the request ourselves:

redirected_url = urllib.parse.urljoin(url, e.headers['Location'])
resp = urllib.request.urlopen(redirected_url)

tomaszferens avatar Jul 26 '22 18:07 tomaszferens

☝🏻 Indeed, this looks like the best approach... just a random observation, Location header value is of the form https://api.github.com/repositories/347746667/pulls (so using the numeric id, not org/repo format).

Again, for the implementer: a ready repo to test on could be e.g. https://github.com/VirtusLab/akka-safer-serializer (renamed to akka-serialization-helper):

$ curl -v -XPOST https://api.github.com/repos/VirtusLab/akka-safer-serializer/pulls -d'{}'
...
< HTTP/2 307 
....
< location: https://api.github.com/repositories/347746667/pulls
...

PawelLipski avatar Jul 26 '22 18:07 PawelLipski

To be released soon in v3.12.0...

PawelLipski avatar Aug 17 '22 10:08 PawelLipski