Refactor BaseEnvironment

readthedocs/doc_builder/environments.py

@@ -77,7 +77,7 @@ class BuildCommand(BuildCommandResultMixin):
 
     def __init__(self, command, cwd=None, shell=False, environment=None,
                  combine_output=True, input_data=None, build_env=None,
-                 bin_path=None, description=None, record_as_success=False):
+                 bin_path=None, description=None):
         self.command = command
         self.shell = shell
         if cwd is None:
@@ -100,7 +100,6 @@ def __init__(self, command, cwd=None, shell=False, environment=None,
         self.description = ''
         if description is not None:
             self.description = description
-        self.record_as_success = record_as_success
         self.exit_code = None
 
     def __str__(self):
@@ -208,13 +207,13 @@ def get_command(self):
 
     def save(self):
         """Save this command and result via the API."""
+        # TODO adap this code to the new one
         # Force record this command as success to avoid Build reporting errors
         # on commands that are just for checking purposes and do not interferes
         # in the Build
         if self.record_as_success:
             log.warning('Recording command exit_code as success')
             self.exit_code = 0
-
         data = {
             'build': self.build_env.build.get('id'),
             'command': self.get_command(),
@@ -318,63 +317,56 @@ def __init__(self, project, environment=None):
         self.environment = environment or {}
         self.commands = []
 
-    def record_command(self, command):
+    def pre_run_command(self, cmd, warn_only, kwargs):
+        """
+        Method to be called before the command is executed.
+
+        The command that will be executed can be accessed as
+        the last element of ``self.commands``.
+        """
+        pass
+
+    def post_run_command(self):
+        """
+        Method to be called after the command is executed.
+
+        The command that was executed can be accessed as
+        the last element of ``self.commands``.
+        """
         pass
 
     def run(self, *cmd, **kwargs):
         """Shortcut to run command from environment."""
         return self.run_command_class(cls=self.command_class, cmd=cmd, **kwargs)
 
-    def run_command_class(
-            self, cls, cmd, record=None, warn_only=False,
-            record_as_success=False, **kwargs):
+    def run_command_class(self, cls, cmd, warn_only=False, **kwargs):
         """
         Run command from this environment.
 
         :param cls: command class to instantiate a command
         :param cmd: command (as a list) to execute in this environment
-        :param record: whether or not to record this particular command
-            (``False`` implies ``warn_only=True``)
         :param warn_only: don't raise an exception on command failure
-        :param record_as_success: force command ``exit_code`` to be saved as
-            ``0`` (``True`` implies ``warn_only=True`` and ``record=True``)
         """
-        if record is None:
-            # ``self.record`` only exists when called from ``*BuildEnvironment``
-            record = getattr(self, 'record', False)
-
-        if not record:
-            warn_only = True
-
-        if record_as_success:
-            record = True
-            warn_only = True
-            # ``record_as_success`` is needed to instantiate the BuildCommand
-            kwargs.update({'record_as_success': record_as_success})
-
         # Remove PATH from env, and set it to bin_path if it isn't passed in
         env_path = self.environment.pop('BIN_PATH', None)
         if 'bin_path' not in kwargs and env_path:
             kwargs['bin_path'] = env_path
         assert 'environment' not in kwargs, "environment can't be passed in via commands."
         kwargs['environment'] = self.environment
 
+        self.pre_run_command(cmd, warn_only, kwargs)
         # ``build_env`` is passed as ``kwargs`` when it's called from a
         # ``*BuildEnvironment``
         build_cmd = cls(cmd, **kwargs)
         build_cmd.run()
 
-        if record:
-            # TODO: I don't like how it's handled this entry point here since
-            # this class should know nothing about a BuildCommand (which are the
-            # only ones that can be saved/recorded)
-            self.record_command(build_cmd)
-
-            # We want append this command to the list of commands only if it has
-            # to be recorded in the database (to keep consistency) and also, it
-            # has to be added after ``self.record_command`` since its
-            # ``exit_code`` can be altered because of ``record_as_success``
-            self.commands.append(build_cmd)
+        # TODO adap this code to the new one
+        # We want append this command to the list of commands only if it has
+        # to be recorded in the database (to keep consistency) and also, it
+        # has to be added after ``self.record_command`` since its
+        # ``exit_code`` can be altered because of ``record_as_success``
+        self.commands.append(build_cmd)
+        self.post_run_command()
 
         if build_cmd.failed:
             msg = u'Command {cmd} failed'.format(cmd=build_cmd.get_command())
@@ -393,13 +385,35 @@ def run_command_class(
         return build_cmd
 
 
-class LocalEnvironment(BaseEnvironment):
+class EnvironmentRecordCommandMixin(object):
+
+    """
+    Allows to save a command to the database.
+
+    It accepts `record` as kwarg. If `record` is True,
+    the command will be saved.
+    If warn_only is True, the command will be recorded as success.
+    """
+
+    def pre_run_command(self, cmd, warn_only, kwargs):
+        self.record = kwargs.pop('record', True)
+        self.record_as_success = warn_only
+
+    def post_run_command(self):
+        command = self.commands[-1]
+        if self.record:
+            if self.record_as_success:
+                command.exit_code = 0
+            command.save()
+
+
+class LocalEnvironment(EnvironmentRecordCommandMixin, BaseEnvironment):
 
     # TODO: BuildCommand name doesn't make sense here, should be just Command
     command_class = BuildCommand
 
 
-class BuildEnvironment(BaseEnvironment):
+class BuildEnvironment(EnvironmentRecordCommandMixin, BaseEnvironment):
 
     """
     Base build environment.
@@ -514,9 +528,6 @@ def handle_exception(self, exc_type, exc_value, _):
             )
             return True
 
-    def record_command(self, command):
-        command.save()
-
     def run(self, *cmd, **kwargs):
         kwargs.update({
             'build_env': self,