Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2080243005:
appengine, crimson: Use updated router API (Closed)
Descriptionappengine, crimson: Use updated router API
This updates the source code to use the new router package found at
github.com/luci/luci-go/server/router. Also, updates luci-go version in DEPS.
BUG=617774
Committed: https://chromium.googlesource.com/infra/infra/+/df8b068e9123402501ecd8bcf9db4b7458d96547
Patch Set 1 #Patch Set 2 : Update infra luci-go deps #
Total comments: 6
Patch Set 3 : Address Vadim's comments #Patch Set 4 : Rebase #
Messages
Total messages: 34 (19 generated)
Description was changed from ========== appengine, crimson: Use updated router API This updates the source code to use the new router package found at github.com/luci/luci-go/server/router. BUG= ========== to ========== DO NOT MERGE. (Need to update luci-go pinned version in DEPS.) appengine, crimson: Use updated router API This updates the source code to use the new router package found at github.com/luci/luci-go/server/router. BUG= ==========
Description was changed from ========== DO NOT MERGE. (Need to update luci-go pinned version in DEPS.) appengine, crimson: Use updated router API This updates the source code to use the new router package found at github.com/luci/luci-go/server/router. BUG= ========== to ========== DO NOT MERGE. (Need to update luci-go pinned version in DEPS.) appengine, crimson: Use updated router API This updates the source code to use the new router package found at github.com/luci/luci-go/server/router. BUG=617774 ==========
nishanths@google.com changed reviewers: + vadimsh@chromium.org
nishanths@google.com changed reviewers: + estaab@chromium.org
Description was changed from ========== DO NOT MERGE. (Need to update luci-go pinned version in DEPS.) appengine, crimson: Use updated router API This updates the source code to use the new router package found at github.com/luci/luci-go/server/router. BUG=617774 ========== to ========== appengine, crimson: Use updated router API This updates the source code to use the new router package found at github.com/luci/luci-go/server/router. BUG=617774 ==========
nishanths@google.com changed reviewers: + nodir@chromium.org
The CQ bit was checked by nishanths@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2080243005/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. Committers are members of the group "project-infra-committers". Note that this has nothing to do with OWNERS files.
The CQ bit was unchecked by nishanths@google.com
https://codereview.chromium.org/2080243005/diff/20001/go/src/infra/appengine/... File go/src/infra/appengine/sheriff-o-matic/main.go (right): https://codereview.chromium.org/2080243005/diff/20001/go/src/infra/appengine/... go/src/infra/appengine/sheriff-o-matic/main.go:518: if appengine.IsDevAppServer() { WithPanicCatcher is part of BaseProd now, so this branch not needed anymore. https://codereview.chromium.org/2080243005/diff/20001/go/src/infra/appengine/... go/src/infra/appengine/sheriff-o-matic/main.go:523: middleware.WithPanicCatcher, here too https://codereview.chromium.org/2080243005/diff/20001/go/src/infra/crimson/se... File go/src/infra/crimson/server/frontend/handler.go (right): https://codereview.chromium.org/2080243005/diff/20001/go/src/infra/crimson/se... go/src/infra/crimson/server/frontend/handler.go:75: logging.Errorf(c.Context, "Failed to get login URL") (I know it was like that before, but) return HTTP 500 in this case.
Description was changed from ========== appengine, crimson: Use updated router API This updates the source code to use the new router package found at github.com/luci/luci-go/server/router. BUG=617774 ========== to ========== appengine, crimson: Use updated router API This updates the source code to use the new router package found at github.com/luci/luci-go/server/router. Also, updates luci-go version in DEPS. BUG=617774 ==========
https://codereview.chromium.org/2080243005/diff/20001/go/src/infra/appengine/... File go/src/infra/appengine/sheriff-o-matic/main.go (right): https://codereview.chromium.org/2080243005/diff/20001/go/src/infra/appengine/... go/src/infra/appengine/sheriff-o-matic/main.go:518: if appengine.IsDevAppServer() { On 2016/06/22 19:50:17, Vadim Sh. wrote: > WithPanicCatcher is part of BaseProd now, so this branch not needed anymore. Done. https://codereview.chromium.org/2080243005/diff/20001/go/src/infra/appengine/... go/src/infra/appengine/sheriff-o-matic/main.go:523: middleware.WithPanicCatcher, On 2016/06/22 19:50:17, Vadim Sh. wrote: > here too Done. https://codereview.chromium.org/2080243005/diff/20001/go/src/infra/crimson/se... File go/src/infra/crimson/server/frontend/handler.go (right): https://codereview.chromium.org/2080243005/diff/20001/go/src/infra/crimson/se... go/src/infra/crimson/server/frontend/handler.go:75: logging.Errorf(c.Context, "Failed to get login URL") On 2016/06/22 19:50:17, Vadim Sh. wrote: > (I know it was like that before, but) return HTTP 500 in this case. Done.
lgtm, thanks!
The CQ bit was checked by nishanths@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2080243005/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Infra Linux Precise 32 Tester on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Linux%20Precise...) Infra Linux Trusty 64 Tester on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Linux%20Trusty%...) Infra Presubmit on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Presubmit/build...) Infra Win Tester on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Win%20Tester/bu...)
The CQ bit was checked by nishanths@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2080243005/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Infra Linux Trusty 64 Tester on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Linux%20Trusty%...) Infra Presubmit on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Presubmit/build...)
You need to rebase your CL on top of new 'master' and reupload it with 'git cl upload'. DEPS has changed there.
The CQ bit was checked by nishanths@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2080243005/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by nishanths@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from vadimsh@chromium.org Link to the patchset: https://codereview.chromium.org/2080243005/#ps60001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2080243005/60001
Message was sent while issue was closed.
Description was changed from ========== appengine, crimson: Use updated router API This updates the source code to use the new router package found at github.com/luci/luci-go/server/router. Also, updates luci-go version in DEPS. BUG=617774 ========== to ========== appengine, crimson: Use updated router API This updates the source code to use the new router package found at github.com/luci/luci-go/server/router. Also, updates luci-go version in DEPS. BUG=617774 Committed: https://chromium.googlesource.com/infra/infra/+/df8b068e9123402501ecd8bcf9db4... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/infra/infra/+/df8b068e9123402501ecd8bcf9db4... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
