diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index 6a2dd4d71f6..5e6bef50114 100644 --- a/readthedocs/doc_builder/environments.py +++ b/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,40 +317,36 @@ 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: @@ -359,22 +354,19 @@ def run_command_class( 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,