New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 784262 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
OoO until Feb 4th
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[WPT Import] Importer fails to use fallback TBR when no one is on ecosystem-infra rotation

Project Member Reported by robertma@chromium.org, Nov 13 2017

Issue description

In the following run:
https://build.chromium.org/p/chromium.infra.cron/builders/wpt-importer/builds/5168
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.infra.cron%2Fwpt-importer%2F5168%2F%2B%2Frecipes%2Fsteps%2FImport_changes_from_WPT_to_Chromium%2F0%2Fstdout

No one is on rotation during weekends, and the importer is supposed to fallback to TBR qyearsley in such case but it didn't.

Most likely because we return '' in _fetch_ecosystem_infra_sheriff_username() in some cases: https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py?q=test_importer&sq=package:chromium&l=513 but the caller only falls back when an exception is raised.
 
I just manually +1 and submitted that CL by the way https://chromium-review.googlesource.com/c/chromium/src/+/764680

So don't be surprised if nothing seems to be broken.
Owner: raphael....@intel.com
Status: Started (was: Available)
That's a regression introduced by https://chromium-review.googlesource.com/c/chromium/src/+/744011; Quinten suggested a possible simplification in tbr_reviewer() and we both overlooked this side-effect.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81645448b527b4953066a551a1f61ddbae7ef425

commit 81645448b527b4953066a551a1f61ddbae7ef425
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Mon Nov 13 17:23:19 2017

wpt import: Never return an empty string in tbr_reviewer().

After commit a565e32 ("WPT import: Stop appending @chromium.org to TBR
emails altogether"), whenever _fetch_ecosystem_infra_sheriff_username()
returned an empty string we'd pass it on instead of falling back to
'qyearsley' like we did before.

Fix it by making sure we always return the fallback string both if there's
an exception as well as if there is nobody currently on rotation.

Bug:  755753 ,  784262 
Change-Id: I31b298bd3ffe53fc10fec42f8096f04d9fdb08dd
Reviewed-on: https://chromium-review.googlesource.com/765492
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#515971}
[modify] https://crrev.com/81645448b527b4953066a551a1f61ddbae7ef425/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py
[modify] https://crrev.com/81645448b527b4953066a551a1f61ddbae7ef425/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py

Status: Fixed (was: Started)

Sign in to add a comment