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

httplib2.RedirectMissingLocation error with GCloud Upload #156

Closed
neilodonuts opened this issue Jan 21, 2020 · 14 comments
Closed

httplib2.RedirectMissingLocation error with GCloud Upload #156

neilodonuts opened this issue Jan 21, 2020 · 14 comments
Assignees

Comments

@neilodonuts
Copy link

@neilodonuts neilodonuts commented Jan 21, 2020

See: https://stackoverflow.com/questions/59815620/gcloud-upload-httplib2-redirectmissinglocation-redirected-but-the-response-is-m

I'm assuming this is a strict change for the better but what are the code changes required to prevent this exception from being thrown?

@neilodonuts
Copy link
Author

@neilodonuts neilodonuts commented Jan 21, 2020

This change caused this issue to start happening: 45441b2#diff-72a22fe2efc9771bbe023ba1ffa07173

The return status causes a check for the location header but apparently this is not coming back from the Google Cloud Storage API. I can't tell at this point if the return type (SAFE_METHODS = ("GET", "HEAD", "OPTIONS", "TRACE")) is causing the issue or the HTTP status code 308

I'll see if I can't dig in further

@neilodonuts
Copy link
Author

@neilodonuts neilodonuts commented Jan 21, 2020

I'm seeing this from a call to Google Cloud Storage API using the old auth2client:

resp = service_object.objects().insert(bucket=bucket, name=key, media_body=file).execute()

The response object from this shows it's a 308 with no location:

{'content-type': 'text/plain; charset=utf-8', 'x-guploader-uploadid': 'AEnB2Uo-Kx5OS3msuFsvIywPzgaEb1x0Dz3PDMZjo6_UqGv8omp8X1vy2LLhpPY73uEWV5DAKIl0FeWm4GRnSpcg7VCWy2J9NQ', 'range': 'bytes=0-104857599', 'x-range-md5': '08cf7d380b0ecfb2dac28bfd7bd2beda', 'content-length': '0', 'date': 'Tue, 21 Jan 2020 19:11:28 GMT', 'server': 'UploadServer', 'alt-svc': 'quic=":443"; ma=2592000; v="46,43",h3-Q050=":443"; ma=2592000,h3-Q049=":443"; ma=2592000,h3-Q048=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000', 'status': '308'}

The work-around for me was to make this change in the upload helper function:

service_object._http.follow_redirects = False

I wrapped it in a try/finally to reset the flag to its previous state. I skimmed this StackOverflow post and it implies that the location header may not be present: https://stackoverflow.com/a/37101262/9642 . I understand that for following redirects, some new location is present but the issue then is if a server (even Google) misbehaves, how does the client handle this?

@temoto
Copy link
Member

@temoto temoto commented Jan 21, 2020

@neilodonuts thanks for report.

At this point I think the best course is to pin httplib2==0.15 as temporary workaround.

Permanent solution would likely consist of httplib2 API to reduce redirect codes set and then gcloud using it.

Disabling redirects may cause more issues if server uses 301/302 responses during normal operation.

@temoto temoto self-assigned this Jan 21, 2020
@temoto
Copy link
Member

@temoto temoto commented Jan 21, 2020

If you can afford patching httplib2 source, change this line:

            if self.follow_redirects and response.status in (300, 301, 302, 303, 307, 308):

to remove 308 from tuple.

@temoto
Copy link
Member

@temoto temoto commented Jan 22, 2020

TL;DR

gcloud package is deprecated twice and is not compatible with httplib2>=0.16. Proper solution is to use google-cloud-* packages family.

Iff that's not possible, edit your requirements.txt to pin down httplib2<0.16.0


Long story

google cloud storage server uses HTTP 308 for special resumable uploads feature which somewhat resembles "retry same method to same location", but not quite.

Above is (probably) rationale for PyPI package google-resumable-media which is used by more recent incarnations of gcloud related packages and handles 200 and 308 in similar manner, unlike generic HTTP client should.

History context:

Sorry for bad news. As HTTP enthusiast, I'm biased towards 308 support. Please reach if you have better idea how to handle this situation more gracefully.

@neilodonuts
Copy link
Author

@neilodonuts neilodonuts commented Jan 22, 2020

I understand and agree that the long term is to upgrade to use Google's supported PyPI package. The problem I face myself is a very large inherited code base with a great many unknowns and even small changes have very large effects.

One potential solution would be to have a set that specifies which redirect codes are to be followed. Then for this particular situation, I could do something like this:

_http.follow_redirect_codes = set(_http.follow_redirect_codes) - {308}

In order to save memory the follow_redirect_codes could default to a global frozenset().

temoto added a commit that referenced this issue Jan 23, 2020
…s check

#156
@temoto
Copy link
Member

@temoto temoto commented Jan 23, 2020

@neilodonuts yeah please try this ea0e1f1

@neilodonuts
Copy link
Author

@neilodonuts neilodonuts commented Jan 23, 2020

I tested the change and it worked; I was able to prevent the exception:

_http.redirect_codes = set(_http.redirect_codes) - {308}

I think once this is released, it will help those with this same issue and they won't have to version lock to 0.15.0 or earlier.

@busunkim96
Copy link

@busunkim96 busunkim96 commented Jan 24, 2020

Hi!

Folks filed googleapis/google-api-python-client#803 as 0.16.0 led to broken behavior for uploads. It looks like 308 is used as 'Resume Incomplete' in the Google Drive API and the Youtube API (there may be others that I missed). I doubt it'll be possible for these APIs to change their behavior in the near future, so we will probably also opt to exclude 308s.

Many thanks to @neilodonuts for troubleshooting and @temoto for the quick patch. 👍

@temoto
Copy link
Member

@temoto temoto commented Jan 24, 2020

@neilodonuts it's released v0.17.0

@busunkim96 if it would be possible to change googleapis library to use this redirect_codes workaround, please let me know so I'd update stackoverflow answer with better way forward solution.

clrpackages added a commit to clearlinux-pkgs/httplib2 that referenced this issue Jan 27, 2020
… 0.17.0

commit 67463426cf79af766176b1f318fefefd3d5edc23
Author: Sergey Shepelev <temotor@gmail.com>
Date:   Fri Jan 24 16:57:26 2020 +0300

    v0.17.0 release

commit ea0e1f151bae5fb4bb58629ecf2909f277cef761
Author: Sergey Shepelev <temotor@gmail.com>
Date:   Thu Jan 23 12:25:01 2020 +0300

    feature: Http().redirect_codes set, works after follow(_all)_redirects check

    httplib2/httplib2#156
@tmknight
Copy link

@tmknight tmknight commented Jan 30, 2020

Unfortunately, I still experience the issue on a 4GB file: Windows Python 3.8.1 (x64), Ubuntu 18.04 LTS Python 3.5.2; httplib2==0.17.0

image
httplib2.RedirectMissingLocation: Redirected but the response is missing a Location: header.

Let me know if I can provide additional info or if there is some other pre-req I need for 0.17.0 to work.

Cheers

@temoto
Copy link
Member

@temoto temoto commented Jan 30, 2020

@tmknight did you set redirect_codes ?

@tmknight
Copy link

@tmknight tmknight commented Jan 30, 2020

Ah, I read the commit incorrectly - I see now. Thanks for setting me straight...

@temoto
Copy link
Member

@temoto temoto commented Mar 11, 2020

google-api-python-client has just merged fix using redirect_codes API.

I don't think there is anything else to do, but please comment/reopen freely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants