Adjust master_gen.PopulateBuildmasterConfig() to look at master_site_config

scripts/master/master_gen.py

Index: scripts/master/master_gen.py
diff --git a/scripts/master/master_gen.py b/scripts/master/master_gen.py
index 373b928cc9fa81ef8faf182cb9d5c9d2b5b6c765..ee4f13eb5693e507bded04f9c21b513ecacaddbc 100644
--- a/scripts/master/master_gen.py
+++ b/scripts/master/master_gen.py
@@ -19,35 +19,19 @@ from master.factory import annotator_factory
 
 
 def PopulateBuildmasterConfig(BuildmasterConfig, builders_path,
-                              master_cls=None):
+                              active_master_cls):
   """Read builders_path and populate a build master config dict."""
-  master_cls = master_cls or Master
   builders = chromium_utils.ReadBuildersFile(builders_path)
-  _Populate(BuildmasterConfig, builders, master_cls)
+  _Populate(BuildmasterConfig, builders, active_master_cls)
 
 
-
-def _Populate(BuildmasterConfig, builders, master_cls):
-  classname = builders['master_classname']
-  base_class = getattr(master_cls, builders['master_base_class'])
-  active_master_cls = type(classname, (base_class,), {
-      'buildbot_url': builders['buildbot_url'],
-      'project_name': builders['master_classname'],
-      'master_port': int(builders['master_port']),
-      'master_port_alt': int(builders['master_port_alt']),
-      'slave_port': int(builders['slave_port']),
-      })
-
-  # TODO: Modify this and the factory call, below, so that we can pass the
-  # path to the builders.pyl file through the annotator to the slave so that
-  # the slave can get the recipe name and the factory properties dynamically
-  # without needing the master to re-read things.

[Dirk Pranke, 2015/05/21 23:31:29]

luqui@ fixed this TODO in https://codereview.chromium.org/860383008/

+def _Populate(BuildmasterConfig, builders, active_master_cls):
   m_annotator = annotator_factory.AnnotatorFactory()

[nodir, 2015/05/21 23:44:35]

pass active_master_cls as a parameter,
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/master/f...
It is required for cross-master triggering

[Dirk Pranke, 2015/05/21 23:57:31]

Does cross-master triggering require anything else from the buildbot config (or
builders.pyl), or is the rest of it completely handled inside the recipe?

I can add the class here, of course, but I want to make sure we know and can
keep track of what configs are supported.

[nodir, 2015/05/22 00:09:28]

This should be enough. The rest is handled inside annotator engine and recipes
 
Passing it won't do any harm: it just assigns an attribute.
It is used only if someone attempts to trigger a cross-master build, or a
buildbucket-based build triggers a new build

 
   c = BuildmasterConfig
   c['logCompressionLimit'] = False
   c['projectName'] = active_master_cls.project_name
-  c['projectURL'] = master_cls.project_url
+  c['projectURL'] = Master.project_url

[Dirk Pranke, 2015/05/21 23:31:29]

master_cls is only ever not equal to Master in the unit tests, and it turns out
that using Master in the unit tests for this works just fine as well.

[nodir, 2015/05/21 23:44:34]

So all masters have the same project_url?
As a user of builders.pyl, I'd expect to be able to specify in the builders.pyl.

[Dirk Pranke, 2015/05/21 23:57:31]

That is a reasonable request, but the code doesn't currently support it (i.e.,
this change isn't changing that). I would just as soon not add it in this CL, to
avoid confusion. Feel free to file a separate bug/feature request.

[nodir, 2015/05/22 00:09:28]

Acknowledged.

   c['buildbotURL'] = active_master_cls.buildbot_url
 
   # This sets c['db_url'] to the database connect string in found in
@@ -88,7 +72,7 @@ def _Populate(BuildmasterConfig, builders, master_cls):
   # slaves registered to a builder. Remove dupes.
   c['slaves'] = master_utils.AutoSetupSlaves(
       c['builders'],
-      master_cls.GetBotPassword(),
+      Master.GetBotPassword(),

[Dirk Pranke, 2015/05/21 23:31:29]

same comment as line 34.

       missing_recipients=['buildbot@chromium-build-health.appspotmail.com'])
 
   # This does some sanity checks on the configuration.