This is an automated email from the ASF dual-hosted git repository. juergbi pushed a commit to branch juerg/remote-cache-ci in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 7ecbc41b9c5126a8fa47dbd5193a2d29d0aebf49 Author: Jürg Billeter <[email protected]> AuthorDate: Tue Oct 20 16:29:39 2020 +0200 Drop pull-artifact-files config option for remote execution With `pull-artifact-files` disabled, artifact file blobs are completely ignored in cache checks. While this allows remote execution builds without downloading build outputs, it can result in build failures when blobs expire on the server side. It also does not download missing blobs on demand. I.e. `bst artifact checkout` and other commands don't work with `pull-artifact-files` disabled. This commit removes the `pull-artifact-files` config option due to the issues above. It will be replaced by remote cache support, which only downloads blobs as needed but still considers all file blobs in cache checks for reliable builds. It will also support download of missing blobs on demand. --- doc/source/using_config.rst | 12 ------------ src/buildstream/_context.py | 26 +++++--------------------- src/buildstream/_frontend/widget.py | 1 - src/buildstream/_stream.py | 8 +++----- tests/remoteexecution/partial.py | 13 ++++--------- 5 files changed, 12 insertions(+), 48 deletions(-) diff --git a/doc/source/using_config.rst b/doc/source/using_config.rst index acfff95..6e9fe24 100644 --- a/doc/source/using_config.rst +++ b/doc/source/using_config.rst @@ -900,7 +900,6 @@ using the ``remote-execution`` key, like so: .. code:: yaml remote-execution: - pull-artifact-files: True execution-service: url: http://execution.fallback.example.com:50051 instance-name: main @@ -918,16 +917,6 @@ using the ``remote-execution`` key, like so: Attributes '''''''''' -* ``pull-artifact-files`` - - This determines whether you want the artifacts which were built remotely - to be downloaded into the local CAS, so that it is ready for checkout - directly after a built completes. - - If this is set to ``false``, then you will need to download the artifacts - you intend to use with :ref:`bst artifact checkout <invoking_artifact_checkout>` - after your build completes. - * ``execution-service`` A :ref:`service configuration <user_config_remote_execution_service>` specifying @@ -1095,7 +1084,6 @@ the global configuration can be overridden on a per project basis in this projec # remote execution configuration. # remote-execution: - pull-artifact-files: True execution-service: url: http://execution.example.com:50051 instance-name: main diff --git a/src/buildstream/_context.py b/src/buildstream/_context.py index 7a46caa..2c1f25a 100644 --- a/src/buildstream/_context.py +++ b/src/buildstream/_context.py @@ -175,9 +175,6 @@ class Context: # User specified cache quota, used for display messages self.config_cache_quota_string: Optional[str] = None - # Whether to pull the files of an artifact when doing remote execution - self.pull_artifact_files: bool = True - # Whether or not to attempt to pull build trees globally self.pull_buildtrees: Optional[bool] = None @@ -376,10 +373,10 @@ class Context: cache_config = defaults.get_mapping("source-caches", default={}) self._global_source_cache_config = _CacheConfig.new_from_node(cache_config) - # Load the global remote execution config including pull-artifact-files setting + # Load the global remote execution config remote_execution = defaults.get_mapping("remote-execution", default=None) if remote_execution: - self.pull_artifact_files, self.remote_execution_specs = self._load_remote_execution(remote_execution) + self.remote_execution_specs = self._load_remote_execution(remote_execution) # Load pull build trees configuration self.pull_buildtrees = cache.get_bool("pull-buildtrees") @@ -549,7 +546,7 @@ class Context: override_node = self.get_overrides(project.name) remote_execution = override_node.get_mapping("remote-execution", default=None) if remote_execution: - self.pull_artifact_files, self.remote_execution_specs = self._load_remote_execution(remote_execution) + self.remote_execution_specs = self._load_remote_execution(remote_execution) # # Maintain our list of remote specs for artifact and source caches @@ -758,18 +755,5 @@ class Context: if not os.environ.get("XDG_DATA_HOME"): os.environ["XDG_DATA_HOME"] = os.path.expanduser("~/.local/share") - def _load_remote_execution(self, node: MappingNode) -> Tuple[bool, Optional[RemoteExecutionSpec]]: - # The pull_artifact_files attribute is special, it is allowed to - # be set to False even if there is no remote execution service configured. - # - pull_artifact_files: bool = node.get_bool("pull-artifact-files", default=True) - node.safe_del("pull-artifact-files") - - # Don't pass the remote execution settings if that was the only option - remote_execution_specs: Optional[RemoteExecutionSpec] - if node.keys(): - remote_execution_specs = RemoteExecutionSpec.new_from_node(node) - else: - remote_execution_specs = None - - return pull_artifact_files, remote_execution_specs + def _load_remote_execution(self, node: MappingNode) -> Optional[RemoteExecutionSpec]: + return RemoteExecutionSpec.new_from_node(node) diff --git a/src/buildstream/_frontend/widget.py b/src/buildstream/_frontend/widget.py index 589e548..9994818 100644 --- a/src/buildstream/_frontend/widget.py +++ b/src/buildstream/_frontend/widget.py @@ -500,7 +500,6 @@ class LogLine(Widget): values["Storage Service"] = format_spec(specs.storage_spec) if specs.action_spec: values["Action Cache Service"] = format_spec(specs.action_spec) - values["Pull artifact files"] = context.pull_artifact_files text += self._format_values(values) # Print information about each loaded project diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py index 520483b..0559d8d 100644 --- a/src/buildstream/_stream.py +++ b/src/buildstream/_stream.py @@ -383,11 +383,9 @@ class Stream: # Require artifact files only for target elements and their runtime dependencies. self._context.set_artifact_files_optional() - # fetch blobs of targets if options set - if self._context.pull_artifact_files: - scope = _Scope.ALL if selection == _PipelineSelection.ALL else _Scope.RUN - for element in self.targets: - element._set_artifact_files_required(scope=scope) + scope = _Scope.ALL if selection == _PipelineSelection.ALL else _Scope.RUN + for element in self.targets: + element._set_artifact_files_required(scope=scope) source_push_enabled = self._sourcecache.has_push_remotes() diff --git a/tests/remoteexecution/partial.py b/tests/remoteexecution/partial.py index a47abee..cd87293 100644 --- a/tests/remoteexecution/partial.py +++ b/tests/remoteexecution/partial.py @@ -20,9 +20,8 @@ DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "project") # Test that `bst build` does not download file blobs of a build-only dependency # to the local cache. @pytest.mark.datafiles(DATA_DIR) [email protected]("pull_artifact_files", [True, False]) @pytest.mark.parametrize("build_all", [True, False]) -def test_build_dependency_partial_local_cas(cli, datafiles, pull_artifact_files, build_all): +def test_build_dependency_partial_local_cas(cli, datafiles, build_all): project = str(datafiles) element_name = "no-runtime-deps.bst" builddep_element_name = "autotools/amhello.bst" @@ -35,24 +34,20 @@ def test_build_dependency_partial_local_cas(cli, datafiles, pull_artifact_files, # configure pull blobs if build_all: cli.configure({"build": {"dependencies": "all"}}) - cli.config["remote-execution"]["pull-artifact-files"] = pull_artifact_files result = cli.run(project=project, args=["build", element_name]) result.assert_success() # Verify artifact is pulled bar files when ensure artifact files is set result = cli.run(project=project, args=["artifact", "checkout", element_name, "--directory", checkout]) - if pull_artifact_files: - result.assert_success() - assert_contains(checkout, ["/test"]) - else: - result.assert_main_error(ErrorDomain.STREAM, "uncached-checkout-attempt") + result.assert_success() + assert_contains(checkout, ["/test"]) # Verify build dependencies is pulled for ALL and BUILD result = cli.run( project=project, args=["artifact", "checkout", builddep_element_name, "--directory", builddep_checkout] ) - if build_all and pull_artifact_files: + if build_all: result.assert_success() else: result.assert_main_error(ErrorDomain.STREAM, "uncached-checkout-attempt")
