Raise exceptions properly on HTTP errors from OAuthRpcServer (which is only used on bots)

Raise exceptions properly on HTTP errors from OAuthRpcServer (which is only used on bots)

This will hopefully make Rietveld._send retry 500s like it promises to

BUG=585632
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298726

[dsansome, 2016-02-10 02:44:23]

Description was changed from

==========
Raise exceptions properly on HTTP errors from OAuthRpcServer (which is only used
on bots)

This will hopefully make Rietveld._send retry 500s like it promises to

BUG=
==========

to

==========
Raise exceptions properly on HTTP errors from OAuthRpcServer (which is only used
on bots)

This will hopefully make Rietveld._send retry 500s like it promises to

BUG=585632
==========

[dsansome, 2016-02-10 02:45:03]

dsansome@chromium.org changed reviewers:
+ agable@chromium.org, vadimsh@chromium.org

[dsansome, 2016-02-10 02:45:03]

Patch set 1 is the same as https://codereview.chromium.org/1683603002 (that got
rolled back), patch set 2 fixes crbug/585632

[agable, 2016-02-10 17:34:50]

lgtm

[Vadim Sh., 2016-02-10 19:52:09]

https://codereview.chromium.org/1681333005/diff/20001/rietveld.py
File rietveld.py (right):

https://codereview.chromium.org/1681333005/diff/20001/rietveld.py#newcode577
rietveld.py:577: if ret[0].status != 200:
< 300 are usually successes. It's entirely acceptable for Rietveld to return
HTTP 201 on some POST request.

Probably better to do "status >= 500" check or use 'flake_codes' set from above.

[dsansome, 2016-02-11 00:26:29]

https://codereview.chromium.org/1681333005/diff/20001/rietveld.py
File rietveld.py (right):

https://codereview.chromium.org/1681333005/diff/20001/rietveld.py#newcode577
rietveld.py:577: if ret[0].status != 200:
On 2016/02/10 19:52:08, Vadim Sh. wrote:
> < 300 are usually successes. It's entirely acceptable for Rietveld to return
> HTTP 201 on some POST request.
> 
> Probably better to do "status >= 500" check or use 'flake_codes' set from
above.

Good catch, thanks!  I've changed it to >= 300, since if this doesn't raise an
exception the caller will usually try to parse it as json which will fail anyway
for redirect text or an error message.

[dsansome, 2016-02-11 00:26:34]

The CQ bit was checked by dsansome@chromium.org

[dsansome, 2016-02-11 00:26:34]

The patchset sent to the CQ was uploaded after l-g-t-m from agable@chromium.org
Link to the patchset: https://codereview.chromium.org/1681333005/#ps40001
(title: "All 200s are successes")

[commit-bot: I haz the power, 2016-02-11 00:26:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1681333005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1681333005/40001

[commit-bot: I haz the power, 2016-02-11 00:28:40]

Description was changed from

==========
Raise exceptions properly on HTTP errors from OAuthRpcServer (which is only used
on bots)

This will hopefully make Rietveld._send retry 500s like it promises to

BUG=585632
==========

to

==========
Raise exceptions properly on HTTP errors from OAuthRpcServer (which is only used
on bots)

This will hopefully make Rietveld._send retry 500s like it promises to

BUG=585632

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298726
==========

[commit-bot: I haz the power, 2016-02-11 00:28:40]

Committed patchset #3 (id:40001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=298726