Instrument HTTP client requests to buildbucket

Instrument HTTP client requests to buildbucket

BUG=620610
Committed: https://chromium.googlesource.com/chromium/tools/build/+/09978a7ca1b438e5adb63cbbae48f98d8e21b9b0

[dsansome, 2016-08-17 04:33:04]

dsansome@chromium.org changed reviewers:
+ nodir@chromium.org, sergeyberezin@chromium.org

[dsansome, 2016-08-17 04:35:13]

dsansome@chromium.org changed reviewers:
+ stip@chromium.org, vadimsh@chromium.org
- nodir@chromium.org

[dsansome, 2016-08-17 04:35:13]

Oh, nodir's OOO so +some other people.

[nodir, 2016-08-17 07:01:51]

nodir@chromium.org changed reviewers:
+ nodir@chromium.org

[nodir, 2016-08-17 07:01:52]

https://codereview.chromium.org/2254643002/diff/20001/scripts/master/deferred...
File scripts/master/deferred_resource.py (right):

https://codereview.chromium.org/2254643002/diff/20001/scripts/master/deferred...
scripts/master/deferred_resource.py:338: self._th_local.http =
self._resource._http
That doesn't look thread-safe anymore

[dsansome, 2016-08-17 08:55:41]

https://codereview.chromium.org/2254643002/diff/20001/scripts/master/deferred...
File scripts/master/deferred_resource.py (right):

https://codereview.chromium.org/2254643002/diff/20001/scripts/master/deferred...
scripts/master/deferred_resource.py:338: self._th_local.http =
self._resource._http
On 2016/08/17 07:01:52, nodir (OOO till September) wrote:
> That doesn't look thread-safe anymore

Ah so *that's* why it's done this way.  I've changed it to create an
InstrumentedHttp here directly.

Is the Http in the Resource actually used at all?

[Vadim Sh., 2016-08-17 19:22:20]

lgtm if it works

[Sergey Berezin, 2016-08-17 20:10:49]

LGTM, thanks!

I hope it works (it would in infra.git). If only there was a way to test code in
build... :-(

[nodir, 2016-08-17 21:04:31]

On 2016/08/17 20:10:49, Sergey Berezin wrote:
> LGTM, thanks!
> 
> I hope it works (it would in infra.git). If only there was a way to test code
in
> build... :-(

Lgtm, there should be tests in unittests subdirectories

[nodir, 2016-08-17 21:07:29]

https://codereview.chromium.org/2254643002/diff/20001/scripts/master/deferred...
File scripts/master/deferred_resource.py (right):

https://codereview.chromium.org/2254643002/diff/20001/scripts/master/deferred...
scripts/master/deferred_resource.py:338: self._th_local.http =
self._resource._http
On 2016/08/17 08:55:41, dsansome wrote:
> On 2016/08/17 07:01:52, nodir (OOO till September) wrote:
> > That doesn't look thread-safe anymore
> 
> Ah so *that's* why it's done this way. 
Yes, th_local is threading.Local
> I've changed it to create an
> InstrumentedHttp here directly


> Is the Http in the Resource actually used at all?

No

[dsansome, 2016-08-24 03:13:17]

The CQ bit was checked by dsansome@chromium.org

[commit-bot: I haz the power, 2016-08-24 03:13:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 03:21:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-24 03:21:52]

Try jobs failed on following builders:
  Build Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/30d35b66fcdd3b10)

[dsansome, 2016-08-25 07:07:36]

The CQ bit was checked by dsansome@chromium.org

[dsansome, 2016-08-25 07:07:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
sergeyberezin@chromium.org, vadimsh@chromium.org, nodir@chromium.org
Link to the patchset: https://codereview.chromium.org/2254643002/#ps60001
(title: "Fix test include paths")

[commit-bot: I haz the power, 2016-08-25 07:07:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 07:15:45]

Description was changed from

==========
Instrument HTTP client requests to buildbucket

BUG=620610
==========

to

==========
Instrument HTTP client requests to buildbucket

BUG=620610

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/09978a7ca1b438e5adb6...
==========

[commit-bot: I haz the power, 2016-08-25 07:15:46]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/tools/build/+/09978a7ca1b438e5adb6...