[step] Remove config indirection.

recipe_modules/step/api.py

 # Copyright 2013 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 import contextlib
 import copy
+import types
 
 from recipe_engine import recipe_api
+from recipe_engine.config_types import Path
+from recipe_engine.util import Placeholder
 
 
 # Inherit from RecipeApiPlain because the only thing which is a step is
 # run_from_dict()
 class StepApi(recipe_api.RecipeApiPlain):
 
   step_client = recipe_api.RequireClient('step')
 
   def __init__(self, step_properties, **kwargs):
     super(StepApi, self).__init__(**kwargs)
@@ skipping 97 matching lines @@
       infra_step: Whether or not this is an infrastructure step. Infrastructure
         steps will place the step in an EXCEPTION state and raise InfraFailure.
       wrapper: If supplied, a command to prepend to the executed step as a
         command wrapper.
       timeout: If supplied, the recipe engine will kill the step after the
         specified number of seconds.
       allow_subannotations (bool): if True, lets the step emit its own
           annotations. NOTE: Enabling this can cause some buggy behavior. Please
           strongly consider using step_result.presentation instead. If you have
           questions, please contact infra-dev@chromium.org.
       trigger_specs: a list of trigger specifications

[iannucci, 2017/06/12 19:37:57]

do you know enough about this to add a bit more documentation?

[dnj, 2017/06/13 18:28:38]

no :(

       stdout: Placeholder to put step stdout into. If used, stdout won't appear
           in annotator's stdout (and |allow_subannotations| is ignored).
       stderr: Placeholder to put step stderr into. If used, stderr won't appear
           in annotator's stderr.
       stdin: Placeholder to read step stdin from.
       step_test_data (func -> recipe_test_api.StepTestData): A factory which
           returns a StepTestData object that will be used as the default test
           data for this step. The recipe author can override/augment this object
           in the GenTests function.
 
     Returns:
       Opaque step object produced and understood by recipe engine.
     """
-    kwargs = {}
-    if allow_subannotations is not None:
-      kwargs['allow_subannotations'] = allow_subannotations
-    if trigger_specs:
-      kwargs['trigger_specs'] = trigger_specs
-    if stdout:
-      kwargs['stdout'] = stdout
-    if stderr:
-      kwargs['stderr'] = stderr
-    if stdin:
-      kwargs['stdin'] = stdin
-    if step_test_data:
-      kwargs['step_test_data'] = step_test_data
-    assert cmd is None or isinstance(cmd, list)
-    if not ok_ret:
-      ok_ret = {0}
-    if ok_ret in ('any', 'all'):
-      ok_ret = set(range(-256, 256))
-
-    if cmd is not None:
-      command = list(wrapper)
-      command += cmd
-      kwargs['cmd'] = command
-
-    kwargs['timeout'] = timeout
-    kwargs['ok_ret'] = ok_ret
-
     # Calculate our full step name. If a step already has that name, add an
     # index to the end of it.
     #
     # Note that another step could exist with that index already added to it
     # by the user. If this happens, we'll continue appending indexes until we
     # have a unique step name.
     with self.m.context(name_prefix=name):
       base_name = self.m.context.name_prefix
     name_suffix = ''
 
     while True:
       full_name = base_name + name_suffix
       if full_name not in self._seen_steps:
         break
 
       step_count = self._step_names.setdefault(full_name, 1) + 1
       self._step_names[full_name] = step_count
       name_suffix = ' (%d)' % step_count
     self._seen_steps.add(full_name)
 
+    assert isinstance(cmd, (types.NoneType, list))
+    if cmd is not None:
+      cmd = list(wrapper) + cmd
+      assert all(isinstance(x, (int, long, basestring, Path, Placeholder))
+                 for x in cmd)
+
     cwd = self.m.context.cwd
-    if cwd is not None and cwd != self.m.path['start_dir']:
-      kwargs['cwd'] = cwd
-    kwargs['env'] = self.m.context.env
+    if cwd and cwd == self.m.path['start_dir']:
+      cwd = None
+
+    env = self.m.context.env
     if self._prefix_path:
       ps = self.m.path.pathsep
       prefix = ps.join(self._prefix_path)
-      suffix = kwargs['env'].get('PATH', '%(PATH)s')
-      kwargs['env']['PATH'] = '%s%s%s' % (prefix, ps, suffix)
-    kwargs['infra_step'] = self.m.context.infra_step or bool(infra_step)
-    kwargs['step_nest_level'] = self.m.context.nest_level
-    kwargs['name'] = full_name
-    kwargs['base_name'] = name
+      suffix = env.get('PATH', '%(PATH)s')
+      env['PATH'] = '%s%s%s' % (prefix, ps, suffix)
 
-    schema = self.make_config()
-    schema.set_val(kwargs)
-    return self.run_from_dict(schema.as_jsonish())
+    if ok_ret in ('any', 'all'):
+      ok_ret = range(-256, 256)
 
-  # TODO(martiniss) delete, and make generator_script use **kwargs on step()
+    return self.run_step(
+        name=full_name,
+        base_name=full_name or name,
+        cmd=cmd,
+        cwd=cwd,
+        env=env,
+        allow_subannotations=bool(allow_subannotations),
+        trigger_specs=[self._make_trigger_spec(trig)
+                       for trig in (trigger_specs or ())],
+        timeout=timeout,
+        infra_step=self.m.context.infra_step or bool(infra_step),
+        stdout=stdout,
+        stderr=stderr,
+        stdin=stdin,
+        ok_ret=ok_ret,
+        step_test_data=step_test_data,
+        nest_level=self.m.context.nest_level,
+    )
+
   @recipe_api.composite_step
-  def run_from_dict(self, dct):
-    return self.step_client.run_step(dct)
+  def run_step(self, **kwargs):
+    # TODO(dnj,iannucci): Remove this once "generator_script" is deleted. Make
+    # sure we preserve "composite_step".
+    return self.step_client.run_step(self.step_client.StepConfig(**kwargs))

[iannucci, 2017/06/12 19:37:57]

delete this function

[dnj, 2017/06/13 18:28:38]

Done.

+
+  def _make_trigger_spec(self, trig):
+    buildbot_changes = trig.get('buildbot_changes')
+    assert isinstance(buildbot_changes, (types.NoneType, list))
+
+    critical = trig.get('critical')
+    return self.step_client.TriggerSpec(
+        bucket=trig.get('bucket'),
+        builder_name=trig['builder_name'],
+        properties=trig.get('properties'),
+        buildbot_changes=buildbot_changes,
+        tags=trig.get('tags'),
+        critical=bool(critical) if critical is not None else (True),
+    )