[Notes] [Git][BuildStream/buildstream][master] 4 commits: _artifactcache/artifactcache.py: Ensure no double-setup of remotes

[Tristan Van Berkom <gitlab mg gitlab com>]

Title: GitLab

Tristan Van Berkom pushed to branch master at BuildStream / buildstream

Commits:

• 7b117e40
  by Daniel Silverstone at 2018-09-18T08:41:32Z

  _artifactcache/artifactcache.py: Ensure no double-setup of remotes
  
  Since ArtifactCache.setup_remotes() can be expensive and should only
  happen once, this commit will assert() if it is called a second time
  on an artifact cache instance.
  
  Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>
  

• 345f5f49
  by Daniel Silverstone at 2018-09-18T08:41:32Z

  tests/artifactcache/pull.py: Do not double-initialize remotes
  
  The initialization of remotes is done by ArtifactCache.setup_remotes()
  and as such it was wrong for these tests to be calling
  CASCache.initialize_remotes() a second time.
  
  Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>
  

• e32221b6
  by Daniel Silverstone at 2018-09-18T08:45:50Z

  sandbox/_sandboxremote.py: Acquire artifact cache via Platform
  
  The SandboxRemote used to construct its own CASCache which was
  considered dangerous.  This patch replaces that with acquisition of
  the cache via the Platform singleton, hopefully eliminating issues
  from having more than one artifact cache object in a single process.
  
  Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>
  

• b587579f
  by Tristan Van Berkom at 2018-09-18T09:53:26Z

  Merge branch 'danielsilverstone-ct/fix-654' into 'master'
  
  sandbox/_sandboxremote.py: Acquire cache via Platform
  
  See merge request BuildStream/buildstream!797

3 changed files:

• buildstream/_artifactcache/artifactcache.py
• buildstream/sandbox/_sandboxremote.py
• tests/artifactcache/pull.py

Changes:

buildstream/_artifactcache/artifactcache.py

---

...	...	@@ -91,6 +91,7 @@ class ArtifactCache():
91	91	self._cache_size = None # The current cache size, sometimes it's an estimate
92	92	self._cache_quota = None # The cache quota
93	93	self._cache_lower_threshold = None # The target cache size for a cleanup
	94	+ self._remotes_setup = False # Check to prevent double-setup of remotes
94	95
95	96	os.makedirs(self.extractdir, exist_ok=True)
96	97	os.makedirs(self.tmpdir, exist_ok=True)
...	...	@@ -143,6 +144,10 @@ class ArtifactCache():
143	144	#
144	145	def setup_remotes(self, *, use_config=False, remote_url=None):
145	146
	147	+ # Ensure we do not double-initialise since this can be expensive
	148	+ assert(not self._remotes_setup)
	149	+ self._remotes_setup = True
	150	+
146	151	# Initialize remote artifact caches. We allow the commandline to override
147	152	# the user config in some cases (for example `bst push --remote=...`).
148	153	has_remote_caches = False

buildstream/sandbox/_sandboxremote.py

---

...	...	@@ -27,7 +27,7 @@ from . import Sandbox
27	27	from ..storage._filebaseddirectory import FileBasedDirectory
28	28	from ..storage._casbaseddirectory import CasBasedDirectory
29	29	from .._protos.build.bazel.remote.execution.v2 import remote_execution_pb2, remote_execution_pb2_grpc
30		-from .._artifactcache.cascache import CASCache
	30	+from .._platform import Platform
31	31
32	32
33	33	class SandboxError(Exception):
...	...	@@ -43,7 +43,6 @@ class SandboxRemote(Sandbox):
43	43
44	44	def __init__(self, *args, **kwargs):
45	45	super().__init__(*args, **kwargs)
46		- self.cascache = None
47	46
48	47	url = urlparse(kwargs['server_url'])
49	48	if not url.scheme or not url.hostname or not url.port:
...	...	@@ -56,12 +55,6 @@ class SandboxRemote(Sandbox):
56	55
57	56	self.server_url = '{}:{}'.format(url.hostname, url.port)
58	57
59		- def _get_cascache(self):
60		- if self.cascache is None:
61		- self.cascache = CASCache(self._get_context())
62		- self.cascache.setup_remotes(use_config=True)
63		- return self.cascache
64		-
65	58	def run_remote_command(self, command, input_root_digest, working_directory, environment):
66	59	# Sends an execution request to the remote execution server.
67	60	#
...	...	@@ -78,8 +71,8 @@ class SandboxRemote(Sandbox):
78	71	output_files=[],
79	72	output_directories=[self._output_directory],
80	73	platform=None)
81		-
82		- cascache = self._get_cascache()
	74	+ platform = Platform.get_platform()
	75	+ cascache = platform.artifactcache
83	76	# Upload the Command message to the remote CAS server
84	77	command_digest = cascache.push_message(self._get_project(), remote_command)
85	78	if not command_digest or not cascache.verify_digest_pushed(self._get_project(), command_digest):
...	...	@@ -141,7 +134,8 @@ class SandboxRemote(Sandbox):
141	134	if tree_digest is None or not tree_digest.hash:
142	135	raise SandboxError("Output directory structure had no digest attached.")
143	136
144		- cascache = self._get_cascache()
	137	+ platform = Platform.get_platform()
	138	+ cascache = platform.artifactcache
145	139	# Now do a pull to ensure we have the necessary parts.
146	140	dir_digest = cascache.pull_tree(self._get_project(), tree_digest)
147	141	if dir_digest is None or not dir_digest.hash or not dir_digest.size_bytes:
...	...	@@ -176,7 +170,8 @@ class SandboxRemote(Sandbox):
176	170
177	171	upload_vdir.recalculate_hash()
178	172
179		- cascache = self._get_cascache()
	173	+ platform = Platform.get_platform()
	174	+ cascache = platform.artifactcache
180	175	# Now, push that key (without necessarily needing a ref) to the remote.
181	176	vdir_digest = cascache.push_directory(self._get_project(), upload_vdir)
182	177	if not vdir_digest or not cascache.verify_digest_pushed(self._get_project(), vdir_digest):

tests/artifactcache/pull.py

---

...	...	@@ -137,7 +137,6 @@ def _test_pull(user_config_file, project_dir, artifact_dir,
137	137
138	138	# Manually setup the CAS remote
139	139	cas.setup_remotes(use_config=True)
140		- cas.initialize_remotes()
141	140
142	141	if cas.has_push_remotes(element=element):
143	142	# Push the element's artifact
...	...	@@ -274,7 +273,6 @@ def _test_push_tree(user_config_file, project_dir, artifact_dir, artifact_digest
274	273
275	274	# Manually setup the CAS remote
276	275	cas.setup_remotes(use_config=True)
277		- cas.initialize_remotes()
278	276
279	277	if cas.has_push_remotes():
280	278	directory = remote_execution_pb2.Directory()
...	...	@@ -310,7 +308,6 @@ def _test_pull_tree(user_config_file, project_dir, artifact_dir, artifact_digest
310	308
311	309	# Manually setup the CAS remote
312	310	cas.setup_remotes(use_config=True)
313		- cas.initialize_remotes()
314	311
315	312	if cas.has_push_remotes():
316	313	# Pull the artifact using the Tree object

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.