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

Issue 868512 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove bot_whitelist

Project Member Reported by benjhayden@chromium.org, Jul 27

Issue description

The chromeperf dashboard currently has two ways of recording which bots are internal only: bot_whitelist is a list of externally available bot names; then the Bot model contains a boolean "internal_only". There are 11 bot names in bot_whitelist whose Bot entities have internal_only=True, and 29 Bot entities whose internal_only=False but whose names are not in bot_whitelist. I'll share a doc for us to coordinate figuring out which should be which.
There are also many TestMetadata and Anomaly entities whose internal_only doesn't match their Bot.

bot_whitelist is used by add_histograms, add_histograms_queue, add_point_queue, change_internal_only. add_point_queue, add_histograms, add_histograms_queue only use it to check a single Bot, which they could get() instead. change_internal_only could be replaced by dev_console. It’s a confusing UI anyway.

 
Agreed it's confusing UI currently, but what's the end state you have in mind for how one configures whether a bot is internal only?
from google.appengine.ext import ndb

print ndb.Key('Master', master_name, 'Bot', bot_name).get().internal_only

from dashboard.change_internal_only import UpdateBotSync

UpdateBotSync(master_name, bot_name, False)

I don't think I should be the one making a decision, but I'll provide some background. I can't make the services meeting on Wednesday, so I'll dump my thoughts in the bug, but it's fine if a different direction is chosen. 

I think the ideal fix is to:
1) Remove all obsolete bots/masters, and extend deprecate_tests.py to do this automatically in the future.
2) Fix the existing TestMetadata and bot_whitelist to match the bots (making all ChromiumPerf bots public, if they're not already)
3) Fold bot_whitelist into https://chromeperf.appspot.com/change_internal_only
4) Find a clear way to give waterfall owners access to change_internal_only. For example, Juan should be able to change the internal waterfall, Ned and John chromium, Kendal fuchsia, etc. Perhaps this gets updated through HistogramSet, or perhaps there's some simple way to manage owners.

The reason I'd like to do it this way is that we'd been moving towards a more self-serve model for dashboard users. They can update test ownership and histogram relationships through HistogramSet instead of having something hard-coded on the dashboard or stored in the datastore as a source of truth. Simon has a proposal for moving the sheriffing configurations in a similar direction. Making it easy for the owners to see and change which bots are internal-only seems like a move in this direction. Moving it to HistogramSet might be a similar move in this direction.

One thing to note is that data should be internal-only if it's not specified. Most new users of the dashboard are internal-only.
1) So I have an old cl where I did just that, https://chromium-review.googlesource.com/c/catapult/+/786613

Dunno why I didn't cq it, I could do that now.

Yeah maybe the answer is to push this down the pipeline as histogram metadata like everything else.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 24

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/6e337f582634a117a0f2e6f017866e8c6cc95160

commit 6e337f582634a117a0f2e6f017866e8c6cc95160
Author: benshayden <benjhayden@chromium.org>
Date: Fri Aug 24 17:51:35 2018

Remove bot_whitelist.

There are several Bots whose internal_only doesn't match bot_whitelist. They are
all deprecated, so they don't need to be fixed, but having multiple sources of
the same information that aren't automatically updated can cause confusion.

This CL removes bot_whitelist so that the Bot entities are the sole source of truth
for internal_only.

This is split from https://chromium-review.googlesource.com/c/catapult/+/1153585

Bug:  chromium:868512 
Change-Id: Id98f74962fc37adc795cab297159d9d0c7b89937
Reviewed-on: https://chromium-review.googlesource.com/1176274
Commit-Queue: Ben Hayden <benjhayden@chromium.org>
Reviewed-by: Simon Hatch <simonhatch@chromium.org>

[modify] https://crrev.com/6e337f582634a117a0f2e6f017866e8c6cc95160/dashboard/dashboard/change_internal_only_test.py
[modify] https://crrev.com/6e337f582634a117a0f2e6f017866e8c6cc95160/dashboard/dashboard/elements/nav-bar.html
[modify] https://crrev.com/6e337f582634a117a0f2e6f017866e8c6cc95160/dashboard/dashboard/add_histograms_test.py
[modify] https://crrev.com/6e337f582634a117a0f2e6f017866e8c6cc95160/dashboard/dashboard/add_point_queue.py
[modify] https://crrev.com/6e337f582634a117a0f2e6f017866e8c6cc95160/dashboard/dashboard/models/graph_data.py
[modify] https://crrev.com/6e337f582634a117a0f2e6f017866e8c6cc95160/dashboard/dashboard/add_histograms_queue_test.py
[modify] https://crrev.com/6e337f582634a117a0f2e6f017866e8c6cc95160/dashboard/dashboard/add_histograms.py
[modify] https://crrev.com/6e337f582634a117a0f2e6f017866e8c6cc95160/dashboard/dashboard/add_point_test.py
[modify] https://crrev.com/6e337f582634a117a0f2e6f017866e8c6cc95160/dashboard/dashboard/add_histograms_queue.py
[modify] https://crrev.com/6e337f582634a117a0f2e6f017866e8c6cc95160/dashboard/dashboard/change_internal_only.py

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 24

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

commit bd4bed873886cf2edc11c015d7447f448dd0fd4d
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Aug 24 22:19:43 2018

Roll src/third_party/catapult 1e44d066259f..72712b0d6078 (6 commits)

https://chromium.googlesource.com/catapult.git/+log/1e44d066259f..72712b0d6078


git log 1e44d066259f..72712b0d6078 --date=short --no-merges --format='%ad %ae %s'
2018-08-24 zmo@chromium.org Add a script to auto generate telemetry_chrome_test_support build target
2018-08-24 benjhayden@chromium.org Remove bot_whitelist.
2018-08-24 simonhatch@chromium.org Pinpoint - Fix flaky migrate tests
2018-08-24 anthonyalridge@google.com Use real metric data for calculating significant changes.
2018-08-24 anthonyalridge@google.com Introduce display region for statistical test results.
2018-08-24 pasko@chromium.org perf_control.py: Remove TODO for a fixed bug


Created with:
  gclient setdep -r src/third_party/catapult@72712b0d6078

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:872521 , chromium:868512 , chromium:877074 ,chromium:866423,chromium:866423,chromium:397118
TBR=sullivan@chromium.org

Change-Id: I2a744233b5777b9b640ea4fbaff5c9fba95114e6
Reviewed-on: https://chromium-review.googlesource.com/1188518
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#586021}
[modify] https://crrev.com/bd4bed873886cf2edc11c015d7447f448dd0fd4d/DEPS

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 30

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/7a770735549a748a5156a6e27279e6b347b04e0b

commit 7a770735549a748a5156a6e27279e6b347b04e0b
Author: benshayden <benjhayden@chromium.org>
Date: Thu Aug 30 20:22:24 2018

Remove /change_internal_only.

This form is used by admins to control which bots are internal only versus
externally available. It is very infrequently used: about once per quarter.
Uploaders file bugs to ask admins to use it to make bots externally visible.

This CL removes this form in favor of allowing admins to call a function in the
dev console. Uploaders will continue to file bugs to ask admins to control
bots' availability, but now admins will use the dev console instead of the
old confusing form.

This is split from https://chromium-review.googlesource.com/c/catapult/+/1153585
Followup related tasks:
 * Remove bot_whitelist in favor of Bot entities' internal_only property.
 * Optimize performance of ChangeInternalOnly.

BUG= chromium:868512 
Change-Id: I1f839b2c91d42bde98e99829971559d9e912a353
Reviewed-on: https://chromium-review.googlesource.com/1176300
Commit-Queue: Ben Hayden <benjhayden@chromium.org>
Reviewed-by: Simon Hatch <simonhatch@chromium.org>

[delete] https://crrev.com/274a0d3d4b74992356aab57706524f3def4f0dc3/dashboard/dashboard/templates/change_internal_only.html
[modify] https://crrev.com/7a770735549a748a5156a6e27279e6b347b04e0b/dashboard/dashboard/dispatcher.py
[modify] https://crrev.com/7a770735549a748a5156a6e27279e6b347b04e0b/dashboard/dashboard/change_internal_only.py
[modify] https://crrev.com/7a770735549a748a5156a6e27279e6b347b04e0b/dashboard/dashboard/change_internal_only_test.py
[modify] https://crrev.com/7a770735549a748a5156a6e27279e6b347b04e0b/dashboard/dashboard/elements/nav-bar.html

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 30

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

commit cfddd9cb6505ad010a9d3a7cf9254fa645994587
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu Aug 30 22:13:43 2018

Roll src/third_party/catapult d539d9382241..7a770735549a (2 commits)

https://chromium.googlesource.com/catapult.git/+log/d539d9382241..7a770735549a


git log d539d9382241..7a770735549a --date=short --no-merges --format='%ad %ae %s'
2018-08-30 benjhayden@chromium.org Remove /change_internal_only.
2018-08-30 benjhayden@chromium.org .


Created with:
  gclient setdep -r src/third_party/catapult@7a770735549a

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:868512 
TBR=sullivan@chromium.org

Change-Id: If64c1988054ec11e6d8208bedb519b999b5aebf7
Reviewed-on: https://chromium-review.googlesource.com/1197291
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#587805}
[modify] https://crrev.com/cfddd9cb6505ad010a9d3a7cf9254fa645994587/DEPS

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 5

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

commit 6a0856924021cf3fec93c5e7818e6d805929bd6a
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Sep 05 22:29:10 2018

Roll src/third_party/catapult 638bda3a17c0..bcc3e491fb2e (2 commits)

https://chromium.googlesource.com/catapult.git/+log/638bda3a17c0..bcc3e491fb2e


git log 638bda3a17c0..bcc3e491fb2e --date=short --no-merges --format='%ad %ae %s'
2018-09-05 benjhayden@chromium.org Optimize change_internal_only
2018-09-05 anthonyalridge@google.com Add back assignment to all labels.


Created with:
  gclient setdep -r src/third_party/catapult@bcc3e491fb2e

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:868512 ,chromium:866423
TBR=sullivan@chromium.org

Change-Id: I6d171eaf79fe7b80da39bd0a86bf5cdbb1343b0c
Reviewed-on: https://chromium-review.googlesource.com/1208290
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#589031}
[modify] https://crrev.com/6a0856924021cf3fec93c5e7818e6d805929bd6a/DEPS

Status: Fixed (was: Assigned)

Sign in to add a comment