[Notes] [Git][BuildStream/buildstream][Qinusty/skipped-rework] 4 commits

Qinusty pushed to branch Qinusty/skipped-rework at BuildStream / buildstream

Commits:

18d0bfb4

by Tom Pollard at 2018-08-30T19:14:33Z

buildstream/_project.py: Report if project.conf is missing name

Explicitly check that project.conf contains a name. This resolves
the issue of the provenance check from _yaml.py incorrectly reporting
the offending file as the default_config_node projectconfig.yaml
when no name key exists in the pre_config_node dict.

95121148

by Tiago Gomes at 2018-08-30T20:52:29Z

Merge branch 'tpollard/591' into 'master'

buildstream/_project.py: Report if project.conf is missing name

See merge request BuildStream/buildstream!680

775d3fca

by Tristan Van Berkom at 2018-08-31T06:05:32Z

_project.py: Adding FIXME comment to address #591 properly

be720914

by Josh Smith at 2018-08-31T09:38:10Z

Add SkipJob for indicating a skipped activity

SkipJob can now be raised within the context of a timed_activity() to
trigger a SKIPPED message instead of a FAILURE message.

This changes the previous usage of SKIPPED within messages to use the
INFO MessageType (used within cascache.py).

Element._pull() now performs its actions from within a timed_activity()
to match Element._push() and allow for SkipJob to be raised.

"Downloaded artifact <cache key>" has been reworded to "Pulled artifact
<cache key>" for consistency with Element._push().

Changes:

buildstream/_artifactcache/cascache.py

@@ -252,7 +252,7 @@ class CASCache(ArtifactCache):
                  else:
                      self.context.message(Message(
                          None,
 -                        MessageType.SKIPPED,
 +                        MessageType.INFO,
                          "Remote ({}) does not have {} cached".format(
                              remote.spec.url, element._get_brief_display_key())
                      ))
@@ -363,7 +363,7 @@ class CASCache(ArtifactCache):
              if skipped_remote:
                  self.context.message(Message(
                      None,
 -                    MessageType.SKIPPED,
 +                    MessageType.INFO,
                      "Remote ({}) already has {} cached".format(
                          remote.spec.url, element._get_brief_display_key())
                  ))

buildstream/_context.py

@@ -26,7 +26,7 @@ from . import _cachekey
  from . import _signals
  from . import _site
  from . import _yaml
 -from ._exceptions import LoadError, LoadErrorReason, BstError
 +from ._exceptions import LoadError, LoadErrorReason, BstError, SkipJob
  from ._message import Message, MessageType
  from ._profile import Topics, profile_start, profile_end
  from ._artifactcache import ArtifactCache
@@ -406,7 +406,15 @@ class Context():
                  self._push_message_depth(silent_nested)
                  yield
 -            except BstError:
 +            except SkipJob as e:
 +                elapsed = datetime.datetime.now() - starttime
 +                message = Message(unique_id, MessageType.SKIPPED, activity_name, detail=str(e), elapsed=elapsed)
 +                self._pop_message_depth()
 +                self.message(message)
 +                # Return to the scope outside of the context
 +                return
++
 +            except BstError as e:
                  # Note the failure in status messages and reraise, the scheduler
                  # expects an error when there is an error.
                  elapsed = datetime.datetime.now() - starttime

buildstream/_exceptions.py

@@ -90,6 +90,7 @@ class ErrorDomain(Enum):
      APP = 12
      STREAM = 13
      VIRTUAL_FS = 14
 +    SKIP = 15
  # BstError is an internal base exception class for BuildSream
@@ -309,3 +310,8 @@ class StreamError(BstError):
  class AppError(BstError):
      def __init__(self, message, detail=None, reason=None):
          super().__init__(message, detail=detail, domain=ErrorDomain.APP, reason=reason)
++
++
 +class SkipJob(Exception):
 +    def __init__(self, *, detail=None):
 +        super().__init__(detail)

buildstream/_project.py

@@ -398,6 +398,17 @@ class Project():
                  "Project requested format version {}, but BuildStream {}.{} only supports up until format version {}"
                  .format(format_version, major, minor, BST_FORMAT_VERSION))
 +        # FIXME:
 +        #
 +        #   Performing this check manually in the absense
 +        #   of proper support from _yaml.node_get(), this should
 +        #   be removed in favor of a proper accessor function
 +        #   from the _yaml module when #591 is fixed.
 +        #
 +        if self._project_conf.get('name') is None:
 +            raise LoadError(LoadErrorReason.INVALID_DATA,
 +                            "{}: project.conf does not contain expected key '{}'".format(projectfile, 'name'))
++
          # The project name, element path and option declarations
          # are constant and cannot be overridden by option conditional statements
          self.name = _yaml.node_get(pre_config_node, str, 'name')

buildstream/element.py

@@ -84,7 +84,7 @@ import shutil
  from . import _yaml
  from ._variables import Variables
  from ._versions import BST_CORE_ARTIFACT_VERSION
 -from ._exceptions import BstError, LoadError, LoadErrorReason, ImplError, ErrorDomain
 +from ._exceptions import BstError, LoadError, LoadErrorReason, ImplError, ErrorDomain, SkipJob
  from .utils import UtilError
  from . import Plugin, Consistency
  from . import SandboxFlags
@@ -1735,19 +1735,18 @@ class Element(Plugin):
          def progress(percent, message):
              self.status(message)
 -        # Attempt to pull artifact without knowing whether it's available
 -        pulled = self._pull_strong(progress=progress)
 +        display_key = self._get_brief_display_key()
 +        with self.timed_activity("Pulling artifact {}".format(display_key)):
 +            # Attempt to pull artifact without knowing whether it's available
 +            pulled = self._pull_strong(progress=progress)
 -        if not pulled and not self._cached() and not context.get_strict():
 -            pulled = self._pull_weak(progress=progress)
 +            if not pulled and not self._cached() and not context.get_strict():
 +                pulled = self._pull_weak(progress=progress)
 -        if not pulled:
 -            return False
 +            if not pulled:
 +                raise SkipJob()
 -        # Notify successfull download
 -        display_key = self._get_brief_display_key()
 -        self.info("Downloaded artifact {}".format(display_key))
 -        return True
 +        return pulled
      # _skip_push():
+     #
@@ -1785,16 +1784,17 @@ class Element(Plugin):
              self.warn("Not pushing tainted artifact.")
              return False
 +        pushed = False
          display_key = self._get_brief_display_key()
          with self.timed_activity("Pushing artifact {}".format(display_key)):
              # Push all keys used for local commit
              pushed = self.__artifacts.push(self, self.__get_cache_keys_for_commit())
              if not pushed:
 -                return False
 +                raise SkipJob()
              # Notify successful upload
              self.info("Pushed artifact {}".format(display_key))
 -            return True
 +        return pushed
      # _shell():
+     #

tests/frontend/pull.py

@@ -356,4 +356,5 @@ def test_pull_missing_notifies_user(caplog, cli, tmpdir, datafiles):
          assert not result.get_pulled_elements(), \
              "No elements should have been pulled since the cache was empty"
 -        assert "SKIPPED Remote ({}) does not have".format(share.repo) in result.stderr
 +        assert "INFO    Remote ({}) does not have".format(share.repo) in result.stderr
 +        assert "SKIPPED Pulling artifact" in result.stderr

tests/testutils/runcli.py

@@ -178,7 +178,7 @@ class Result():
          return list(pushed)
      def get_pulled_elements(self):
 -        pulled = re.findall(r'\[\s*pull:(\S+)\s*\]\s*INFO\s*Downloaded artifact', self.stderr)
 +        pulled = re.findall(r'\[\s*pull:(\S+)\s*\]\s*INFO\s*Pulled artifact', self.stderr)
          if pulled is None:
              return []

[Notes] [Git][BuildStream/buildstream][Qinusty/skipped-rework] 4 commits: buildstream/_project.py: Report if project.conf is missing name

Qinusty pushed to branch Qinusty/skipped-rework at BuildStream / buildstream

Commits:

7 changed files:

Changes: