You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by ju...@apache.org on 2021/05/20 08:09:29 UTC
[buildstream] 04/14: Drop pull-artifact-files config option for
remote execution
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 <j...@bitron.ch>
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)
-@pytest.mark.parametrize("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")