When refreshing credentials with an stream body, rewind the stream be… #174
Conversation
| @@ -554,6 +554,11 @@ def new_request(uri, method='GET', body=None, headers=None, | |||
| else: | |||
| headers['user-agent'] = self.user_agent | |||
|
|
|||
| body_stream_position = None | |||
| if all(hasattr(body, stream_prop) for stream_prop in ('read', | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Please update the commit log message to conform to http://chris.beams.io/posts/git-commit/#seven-rules. |
thobrla
force-pushed
the
body-stream-idem
branch
3 times, most recently
from
8cc4dfc
to
c4104af
Compare
|
It looks like the most recent round is failing because system tests at https://github.com/google/oauth2client/blob/master/scripts/run_system_tests.py#L99 expect environment variables set here https://github.com/google/oauth2client/blob/master/scripts/run_system_tests.sh#L21 - perhaps if those environment variables aren't set, that script shouldn't run? I think the failures are unrelated to my commit. |
|
@dhermes, please take a look at the system test failures in this pull request. |
|
Re-run the build and see if it fails again. (I don't have privileges to re-start on travis-ci.org) No idea why would have fallen through. Let's hope it's an intermittent failure. (I've never seen this in 1500+ builds in Or maybe it was caused by |
|
i agree with travis that the travis failures are unrelated to travis's code. (sorry, couldn't resist.) i tried re-running the tests, to no avail. let's merge this and move the "integration tests are failing" to a new issue? |
|
Tests pass with int or strings; regardless, I'll push the string-conversion change. |
When refreshing credentials, the original request is re-sent after the credentials are refreshed. If the body of that request is a stream, the stream contents are read in the initial request, and the stream must be rewound before the request is re-sent. Otherwise, the original message body will be different (because stream data was skipped).
|
@craigcitro Can you check which environment variables are set in the Travis settings: I can't see because I am not an admin. This still doesn't explain why falls through. |
|
@dhermes those vars seem to be set (at least, there are a handful of relevant-looking names set on that page). let's maybe throw a handful of debugging info into the script (just print some travis-related env vars?) and start a new issue on that one? |
|
@thobrla can you also file an issue to follow up on whether tests should use string or int for status codes? (it'd be fantastic if we could just clean it all up to use the symbolic names in |
|
UPDATE: Our builds have also started failing (after 1500+ builds never failing): https://travis-ci.org/GoogleCloudPlatform/gcloud-python/builds/62434732 It seems the Travis-CI people have changed something (either on purpose or not). I'm going to debug in |
|
@dhermes yes please let's move the why-are-tests-failing discussion out of this unrelated pull request thread. :-) |
|
@nathanielmanistaatgoogle can you merge if you're good with this PR? |
|
Oh right, this whole discussion was actually about making a change... :-P |
Rewind original stream body when refreshing.
…fore re-sending the original request.