Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(310)

Issue 2400053002: Use CBE for firefighter (Closed)

Created:
4 years, 2 months ago by martiniss
Modified:
4 years, 2 months ago
Reviewers:
dtu
CC:
catapult-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -23 lines) Patch
M firefighter/base/constants.py View 1 chunk +1 line, -0 lines 0 comments Download
M firefighter/update/common/buildbot/builds.py View 1 chunk +11 lines, -21 lines 2 comments Download
M firefighter/update/common/buildbot/network.py View 1 chunk +7 lines, -2 lines 0 comments Download

Messages

Total messages: 15 (6 generated)
martiniss
PTAL Not sure if there are tests or anything.... https://codereview.chromium.org/2400053002/diff/1/firefighter/update/common/buildbot/builds.py File firefighter/update/common/buildbot/builds.py (left): https://codereview.chromium.org/2400053002/diff/1/firefighter/update/common/buildbot/builds.py#oldcode70 firefighter/update/common/buildbot/builds.py:70: ...
4 years, 2 months ago (2016-10-06 22:56:33 UTC) #2
dtu
lgtm. There are other calls to BuildUrl in Firefighter too, are those not supported yet? ...
4 years, 2 months ago (2016-10-07 17:10:55 UTC) #3
martiniss
On 2016/10/07 at 17:10:55, dtu wrote: > lgtm. There are other calls to BuildUrl in ...
4 years, 2 months ago (2016-10-07 18:28:47 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2400053002/1
4 years, 2 months ago (2016-10-07 18:28:56 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: Catapult Presubmit on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Presubmit/builds/4682)
4 years, 2 months ago (2016-10-07 18:31:06 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2400053002/1
4 years, 2 months ago (2016-10-07 18:47:29 UTC) #11
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/4bed9fcc4b459147f05107061d741a9b3ca8d56b
4 years, 2 months ago (2016-10-07 18:48:46 UTC) #13
dtu
On 2016/10/07 18:28:47, martiniss wrote: > On 2016/10/07 at 17:10:55, dtu wrote: > > lgtm. ...
4 years, 2 months ago (2016-10-11 22:46:31 UTC) #14
martiniss
4 years, 2 months ago (2016-10-12 01:12:02 UTC) #15
Message was sent while issue was closed.
On 2016/10/11 at 22:46:31, dtu wrote:
> On 2016/10/07 18:28:47, martiniss wrote:
> > On 2016/10/07 at 17:10:55, dtu wrote:
> > > lgtm. There are other calls to BuildUrl in Firefighter too, are those not
> > supported yet?
> > 
> > I wanted to do a simple change first; firefighter does enough fancy json
calls
> > that I just wanted to start with something I knew CBE supported.
> > 
> > I can switch all the other calls over to use CBE/milo once this lands and
works
> > in prod.
> > 
> > > 
> > >
> >
https://codereview.chromium.org/2400053002/diff/1/firefighter/update/common/b...
> > > File firefighter/update/common/buildbot/builds.py (left):
> > > 
> > >
> >
https://codereview.chromium.org/2400053002/diff/1/firefighter/update/common/b...
> > > firefighter/update/common/buildbot/builds.py:70: except (ValueError,
> > urlfetch.ResponseTooLargeError):
> > > On 2016/10/06 22:56:33, martiniss wrote:
> > > > I removed this logic because CBE doesn't support the select logic that
> > you're
> > > > using here. How important is it?
> > > 
> > > Yeah, that's fine. It was only because BuildBot was so slow.
> > 
> > 
> > 
> > Can I get access to the firefighter cloud project?
> 
> Done.

Deployed!

Powered by Google App Engine
This is Rietveld 408576698