Compute Gerrit CL link from buildset tag |
||||||
Issue descriptionBuild https://luci-milo.appspot.com/buildbot/tryserver.v8/v8_linux64_asan_rel_ng_triggered/27229 has buildset tag pointing to a CL. Gerrit sees the Build->CL association, but Milo does not: the page does not display Gerrit CL link. This is because Milo computes CL link from issue, patchset, etc properties. In this case, the build does not need the properties because it does not need to run bot_update. Milo should compute CL link from the buildset tag..
,
Oct 10 2017
Gerrit CL that is showing the build: https://chromium-review.googlesource.com/c/v8/v8/+/707237/2
,
Oct 10 2017
,
Oct 10 2017
That code is specific to LUCI, whereas the build in question is a buildbot build. Milo has two implementations of some things, one for Buildbot and one for LUCI. If the build was on LUCI, it would display the CL link correctly, so I think it shouldn't be blocking the migration. Now that I realize that it is specific to Buildbot, it looks less urgent to me, WDYT?
,
Oct 10 2017
,
Oct 10 2017
If the issue will go away when we'll migrate our tryserver to LUCI, then I guess it's not as urgent and may not even be needed at all. But this will prevent developers from dogfooding Milo until we fully migrate to LUCI.
,
Oct 10 2017
For buildbot, Milo extracts the CL info from properties, the same way the buildbot UI does. There's no reason this shouldn't be working, other than that something didn't get triggered the right way. I agree it's important for devs to have a good experience on Milo, which would make switch between buildbot and luci more seemless. I'll take a quick look.
,
Oct 10 2017
in the logs: Field patch_issue is not a float: "707237" This should be a quick fix.
,
Oct 10 2017
,
Oct 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/infra/luci/luci-go.git/+/70b22a198e2e08f5f9b3927d44fd046e1fcb45ea commit 70b22a198e2e08f5f9b3927d44fd046e1fcb45ea Author: Ryan Tseng <hinoka@google.com> Date: Tue Oct 10 23:48:55 2017 [milo] Fix buildbot gerrit patch surfacing when it is a string Bug: 773306 Change-Id: I08f73a168f13056996281ecc0d5e4a9cfaafe681 Reviewed-on: https://chromium-review.googlesource.com/709938 Commit-Queue: Ryan Tseng <hinoka@chromium.org> Reviewed-by: Nodir Turakulov <nodir@chromium.org> [modify] https://crrev.com/70b22a198e2e08f5f9b3927d44fd046e1fcb45ea/milo/buildsource/buildbot/build.go
,
Nov 8 2017
I guess this is fixed. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by serg...@chromium.org
, Oct 10 2017