|
|
Created:
5 years, 3 months ago by nodir Modified:
5 years, 3 months ago Reviewers:
jrobbins, Vadim Sh., tandrii(chromium) CC:
chromium-reviews, rmistry+cc_chromium.org Base URL:
https://chromium.googlesource.com/infra/infra.git@master Target Ref:
refs/heads/master Project:
infra Visibility:
Public. |
DescriptionRietveld: schedule builds on buildbucket
Builds are scheduled on behalf of the logged in user.
R=jrobbins@chromium.org, vadimsh@chromium.org, tandrii@chromium.org
BUG=461620
Committed: https://chromium.googlesource.com/infra/infra/+/f20032cd06f4f04915e08ada331d65c100525b8f
Patch Set 1 : #
Total comments: 9
Patch Set 2 : addressed all comments #Patch Set 3 : remove -dev #
Created: 5 years, 3 months ago
Messages
Total messages: 15 (6 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #2 (id:60001) has been deleted
Patchset #1 (id:40001) has been deleted
PTAL Buildbucket build [1] was scheduled by Rietveld. See its "user_agent" tag [1]: https://apis-explorer.appspot.com/apis-explorer/?base=https://cr-buildbucket-...
With exact latest changes: Scheduled build: https://apis-explorer.appspot.com/apis-explorer/?base=https://cr-buildbucket-... e3379b2-tainted-dot-chromiumcodereview-hr-staging.appspot.com log: I 2015-09-15 18:08:18.182 200 149 B 1.99 s D 18:08:18.207 I 18:08:18.210 I 18:08:20.171 /250001/edit_flags undefined D 18:08:18.207 Scheduling 1 builds on behalf of nodir@google.com I 18:08:18.210 PUT https://cr-buildbucket-dev.appspot.com/_ah/api/buildbucket/v1/builds/batch I 18:08:20.171 Scheduled buildbucket builds: '9034563659733818336' cr-buildbucket-dev.appspot.com log: I 2015-09-15 18:08:19.961 200 996 B 202 ms I 18:08:20.129 /_ah/spi/BuildBucketApi.put_batch undefined I 18:08:20.129 Build 9034563659733818336 was created by user:nodir@google.com
lgtm % nit https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... File appengine/chromium_rietveld/tests/test_buildbucket.py (right): https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... appengine/chromium_rietveld/tests/test_buildbucket.py:212: email='johndoe@chromium.org') this is not necessary, you set it up in setUp() now.
lgtm, though I'm not familiar with this code https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... File appengine/chromium_rietveld/codereview/buildbucket.py (right): https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... appengine/chromium_rietveld/codereview/buildbucket.py:263: raise BuildBucketError('Could not schedule build(s): %r' % error) how is this displayed to the user? https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... appengine/chromium_rietveld/codereview/buildbucket.py:390: # See tag conventions http://cr-buildbucket.appspot.com/#docs/conventions https://
https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... File appengine/chromium_rietveld/codereview/views_chromium.py (right): https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... appengine/chromium_rietveld/codereview/views_chromium.py:430: if random.random() <= buildbucket_roll_out_percentage: Jason: should we use this or gae version splitting? would it be OK to have version splitting enabled for a couple of days?
https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... File appengine/chromium_rietveld/codereview/views_chromium.py (right): https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... appengine/chromium_rietveld/codereview/views_chromium.py:430: if random.random() <= buildbucket_roll_out_percentage: On 2015/09/17 at 04:23:42, nodir wrote: > Jason: should we use this or gae version splitting? would it be OK to have version splitting enabled for a couple of days? Yes. I think I would favor GAE version splitting because you could adjust the percentage or disable it any time without needing to alter source code. So, let's try that first.
Works on staging https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... File appengine/chromium_rietveld/codereview/buildbucket.py (right): https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... appengine/chromium_rietveld/codereview/buildbucket.py:263: raise BuildBucketError('Could not schedule build(s): %r' % error) On 2015/09/16 21:57:38, Vadim Sh. wrote: > how is this displayed to the user? very rough: on any 500 the old UI shows an alert that does not depend on response body. It is about the same as opening a CL today. We have retries though, so it does not happen often https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... appengine/chromium_rietveld/codereview/buildbucket.py:390: # See tag conventions http://cr-buildbucket.appspot.com/#docs/conventions On 2015/09/16 21:57:38, Vadim Sh. wrote: > https:// Updated all doc links to point to the newer MD doc with https https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... File appengine/chromium_rietveld/codereview/views_chromium.py (right): https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... appengine/chromium_rietveld/codereview/views_chromium.py:430: if random.random() <= buildbucket_roll_out_percentage: On 2015/09/17 06:03:13, Jason Robbins wrote: > On 2015/09/17 at 04:23:42, nodir wrote: > > Jason: should we use this or gae version splitting? would it be OK to have > version splitting enabled for a couple of days? > > Yes. I think I would favor GAE version splitting because you could adjust the > percentage or disable it any time without needing to alter source code. So, > let's try that first. Done. https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... File appengine/chromium_rietveld/tests/test_buildbucket.py (right): https://codereview.chromium.org/1344253002/diff/80001/appengine/chromium_riet... appengine/chromium_rietveld/tests/test_buildbucket.py:212: email='johndoe@chromium.org') On 2015/09/16 11:02:32, tandrii(chromium) wrote: > this is not necessary, you set it up in setUp() now. Done.
The CQ bit was checked by nodir@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from vadimsh@chromium.org, tandrii@chromium.org Link to the patchset: https://codereview.chromium.org/1344253002/#ps120001 (title: "remove -dev")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1344253002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1344253002/120001
Message was sent while issue was closed.
Committed patchset #3 (id:120001) as https://chromium.googlesource.com/infra/infra/+/f20032cd06f4f04915e08ada331d6... |