Reimplemented tree_for_master, which now uses gatekeeper config files to map master URLs to the tre…

infra/tools/builder_alerts/gatekeeper_extras.py

Index: infra/tools/builder_alerts/gatekeeper_extras.py
diff --git a/infra/tools/builder_alerts/gatekeeper_extras.py b/infra/tools/builder_alerts/gatekeeper_extras.py
index c0cd3f831cf932f3cd91a33de37a30c10ce0064e..4bce0f2ddcb83b2ff8e9c7d4794d79850eb7bfc9 100644
--- a/infra/tools/builder_alerts/gatekeeper_extras.py
+++ b/infra/tools/builder_alerts/gatekeeper_extras.py
@@ -6,36 +6,51 @@
 
 import logging
 
+import infra.tools.builder_alerts.buildbot

[agable, 2014/09/04 14:22:31]

Since you only use one thing, could do
from infra.tools.builder_alerts.buildbot import master_name_from_url

[Sergiy Byelozyorov, 2014/09/05 12:59:06]

Done.

 
 def excluded_builders(master_config):
   return master_config[0].get('*', {}).get('excluded_builders', set())
 
 
-# pylint: disable=C0301
-# FIXME: This is currently baked into:
-# https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave/gatekeeper_launch.py
-# http://crbug.com/394961
-MASTER_CONFIG = {
-  'chromium-status': [
-    'chromium',
-    'chromium.chrome',
-    'chromium.chromiumos',
-    'chromium.gpu',
-    'chromium.linux',
-    'chromium.mac',
-    'chromium.memory',
-    'chromium.win',
-  ],
-  'blink-status': [
-    'chromium.webkit',
-  ],
-}
-
-
-def tree_for_master(master_name):
-  for tree_name, master_names in MASTER_CONFIG.items():
-    if master_name in master_names:
+def tree_for_master(master_url, gatekeeper_trees_config):
+  """Returns name of the tree for a given master or its name on failure.

[agable, 2014/09/04 14:22:31]

"Get the name of the tree for a given master url, or the master's name."

[Sergiy Byelozyorov, 2014/09/05 12:59:06]

Done.

+
+  Retuns:

[agable, 2014/09/04 14:22:31]

Don't really need a "returns" section when the first line already starts with
"returns".

[Sergiy Byelozyorov, 2014/09/05 12:59:06]

Done.

+    Tree name of master name if the master url is not present in gatekeeper

[agable, 2014/09/04 14:22:31]

tree name *or*

[Sergiy Byelozyorov, 2014/09/05 12:59:06]

Acknowledged.

+    config.
+  """
+  for tree_name, tree_config in gatekeeper_trees_config.iteritems():
+    if master_url in tree_config['masters']:
       return tree_name
+  return infra.tools.builder_alerts.buildbot.master_name_from_url(master_url)

[agable, 2014/09/04 14:22:31]

Why is this fallback now necessary?

[Sergiy Byelozyorov, 2014/09/05 12:59:07]

Master info may not be in a config file. In this case we return master name, so
that something shows up in sheriff-o-matic.

+
+
+def apply_gatekeeper_rules(alerts, gatekeeper, gatekeeper_trees):
+  filtered_alerts = []
+  for alert in alerts:
+    master_url = alert['master_url']
+    config = gatekeeper.get(master_url)
+    if not config:
+      # Unclear if this should be set or not?
+      # alert['would_close_tree'] = False
+      filtered_alerts.append(alert)
+      continue
+    if alert['builder_name'] in excluded_builders(config):
+      continue
+    alert['would_close_tree'] = would_close_tree(
+        config, alert['builder_name'], alert['step_name'])
+    alert['tree'] = tree_for_master(master_url, gatekeeper_trees)
+    filtered_alerts.append(alert)
+  return filtered_alerts
+
+
+def fetch_master_urls(gatekeeper, args):
+  # Currently using gatekeeper.json, but could use:
+  # https://chrome-infra-stats.appspot.com/_ah/api#p/stats/v1/stats.masters.list

[tandrii(chromium), 2014/09/04 14:21:06]

nit: I thought comments should end with period.

[Sergiy Byelozyorov, 2014/09/05 12:59:07]

Yeah, but URL takes entire line and adding a period would require breaking it
into two lines... and it was this way before in __main__.py (code just moved
from there).

+  master_urls = gatekeeper.keys()
+  if args.master_filter:
+    master_urls = [url for url in master_urls if args.master_filter not in url]
+  return master_urls
 
 
 def would_close_tree(master_config, builder_name, step_name):