[Notes] [Git][BuildStream/buildstream][gokcennurlu/remote_url_override_push_error] Refactor setup_remotes() to remove --remote overriding logic

[Gökçen Nurlu <gitlab mg gitlab com>]

Title: GitLab

Gökçen Nurlu pushed to branch gokcennurlu/remote_url_override_push_error at BuildStream / buildstream

Commits:

• 0c589d39
  by Gökçen Nurlu at 2018-12-21T16:21:23Z

  Refactor setup_remotes() to remove --remote overriding logic
  
  This change simplifies the responsibilities of `setup_remotes()` by extracting
  "getting the default remotes" logic to its own function and moving "--remote"
  related logic to _load(). The latter, especially, helps to remove the need of
  passing the "remote_url" string and "can_push?" boolean to `setup_remotes()`.
  

4 changed files:

• buildstream/_artifactcache/artifactcache.py
• buildstream/_stream.py
• tests/artifactcache/pull.py
• tests/artifactcache/push.py

Changes:

buildstream/_artifactcache/artifactcache.py

---

...	...	@@ -110,37 +110,42 @@ class ArtifactCache():
110	110	# assume project and element names are not allowed to contain slashes
111	111	return '{0}/{1}/{2}'.format(project.name, element_name, key)
112	112
	113	+ # get_remotes_from_projects()
	114	+ #
	115	+ # Generates list artifact caches based on project configuration
	116	+ #
	117	+ # Returns:
	118	+ # (list of (list of ArtifactCacheSpec, Project)): Configurations each are
	119	+ # ready to be consumed by `self._set_remotes()`
	120	+ #
	121	+ # This requires that all of the projects which are to be processed in the session
	122	+ # have already been loaded and are observable in the Context.
	123	+ #
	124	+ def get_remotes_from_projects(self):
	125	+ return [
	126	+ (_configured_remote_artifact_cache_specs(self.context, proj), proj)
	127	+ for proj in self.context.get_projects()
	128	+ ]
	129	+
113	130	# setup_remotes():
114	131	#
115	132	# Sets up which remotes to use
116	133	#
117	134	# Args:
118		- # use_config (bool): Whether to use project configuration
119		- # remote_url (str): Remote artifact cache URL
120		- # push (bool): Whether to enabe push for `remote_url`
	135	+ # remotes (list of (list of ArtifactCacheSpec, Project)): Configurations which are
	136	+ # ready to be consumed by `self._set_remotes()`
121	137	#
122	138	# This requires that all of the projects which are to be processed in the session
123	139	# have already been loaded and are observable in the Context.
124	140	#
125		- def setup_remotes(self, *, use_config=False, remote_url=None, push=False):
126		-
	141	+ def setup_remotes(self, *, remotes=None):
127	142	# Ensure we do not double-initialise since this can be expensive
128	143	assert not self._remotes_setup
129	144	self._remotes_setup = True
130	145
131		- # Initialize remote artifact caches. We allow the commandline to override
132		- # the user config in some cases (for example `bst push --remote=...`).
133		- has_remote_caches = False
134		- if remote_url:
135		- self._set_remotes([ArtifactCacheSpec(remote_url, push=push)])
136		- has_remote_caches = True
137		- if use_config:
138		- for project in self.context.get_projects():
139		- artifact_caches = _configured_remote_artifact_cache_specs(self.context, project)
140		- if artifact_caches: # artifact_caches is a list of ArtifactCacheSpec instances
141		- self._set_remotes(artifact_caches, project=project)
142		- has_remote_caches = True
143		- if has_remote_caches:
	146	+ if remotes:
	147	+ for caches, project in remotes:
	148	+ self._set_remotes(caches, project=project)
144	149	self._initialize_remotes()
145	150
146	151	# specs_from_config_node()

buildstream/_stream.py

---

...	...	@@ -28,6 +28,7 @@ import tarfile
28	28	import tempfile
29	29	from contextlib import contextmanager, suppress
30	30
	31	+from ._artifactcache import ArtifactCacheSpec
31	32	from ._exceptions import StreamError, ImplError, BstError, set_last_task_error
32	33	from ._message import Message, MessageType
33	34	from ._scheduler import Scheduler, SchedStatus, TrackQueue, FetchQueue, BuildQueue, PullQueue, PushQueue
...	...	@@ -934,14 +935,21 @@ class Stream():
934	935	self._pipeline.resolve_elements(track_selected)
935	936	return [], track_selected
936	937
937		- # ArtifactCache.setup_remotes expects all projects to be fully loaded
938		- for project in self._context.get_projects():
939		- project.ensure_fully_loaded()
940		-
	938	+ remotes = []
	939	+ if use_artifact_config:
	940	+ # ArtifactCache.get_remotes_from_projects expects all projects to be
	941	+ # fully loaded
	942	+ for project in self._context.get_projects():
	943	+ project.ensure_fully_loaded()
	944	+ remotes = self._artifacts.get_remotes_from_projects()
	945	+ elif artifact_remote_url is not None:
	946	+ # Build the ArtifactCacheSpec instance based on `--remote`
	947	+ remotes = [(
	948	+ [ArtifactCacheSpec(artifact_remote_url, push=artifact_remote_can_push)],
	949	+ None
	950	+ )]
941	951	# Connect to remote caches, this needs to be done before resolving element state
942		- self._artifacts.setup_remotes(use_config=use_artifact_config,
943		- remote_url=artifact_remote_url,
944		- push=artifact_remote_can_push)
	952	+ self._artifacts.setup_remotes(remotes=remotes)
945	953
946	954	# Now move on to loading primary selection.
947	955	#

tests/artifactcache/pull.py

---

...	...	@@ -146,7 +146,8 @@ def _test_pull(user_config_file, project_dir, artifact_dir,
146	146	element = project.load_elements([element_name])[0]
147	147
148	148	# Manually setup the CAS remote
149		- cas.setup_remotes(use_config=True)
	149	+ remotes = cas.get_remotes_from_projects()
	150	+ cas.setup_remotes(remotes=remotes)
150	151
151	152	if cas.has_push_remotes(element=element):
152	153	# Push the element's artifact
...	...	@@ -284,7 +285,8 @@ def _test_push_tree(user_config_file, project_dir, artifact_dir, artifact_digest
284	285	cas = artifactcache.cas
285	286
286	287	# Manually setup the CAS remote
287		- artifactcache.setup_remotes(use_config=True)
	288	+ remotes = artifactcache.get_remotes_from_projects()
	289	+ artifactcache.setup_remotes(remotes=remotes)
288	290
289	291	if artifactcache.has_push_remotes():
290	292	directory = remote_execution_pb2.Directory()
...	...	@@ -319,7 +321,8 @@ def _test_pull_tree(user_config_file, project_dir, artifact_dir, artifact_digest
319	321	cas = context.artifactcache
320	322
321	323	# Manually setup the CAS remote
322		- cas.setup_remotes(use_config=True)
	324	+ remotes = cas.get_remotes_from_projects()
	325	+ cas.setup_remotes(remotes=remotes)
323	326
324	327	if cas.has_push_remotes():
325	328	# Pull the artifact using the Tree object

tests/artifactcache/push.py

---

...	...	@@ -125,8 +125,8 @@ def _test_push(user_config_file, project_dir, artifact_dir,
125	125	element = project.load_elements([element_name])[0]
126	126
127	127	# Manually setup the CAS remote
128		- cas.setup_remotes(use_config=True)
129		- cas.initialize_remotes()
	128	+ remotes = cas.get_remotes_from_projects()
	129	+ cas.setup_remotes(remotes=remotes)
130	130
131	131	if cas.has_push_remotes(element=element):
132	132	# Push the element's artifact
...	...	@@ -185,8 +185,8 @@ def test_push_directory(cli, tmpdir, datafiles):
185	185	assert artifactcache.contains(element, element_key)
186	186
187	187	# Manually setup the CAS remote
188		- artifactcache.setup_remotes(use_config=True)
189		- artifactcache.initialize_remotes()
	188	+ remotes = artifactcache.get_remotes_from_projects()
	189	+ artifactcache.setup_remotes(remotes=remotes)
190	190	assert artifactcache.has_push_remotes(element=element)
191	191
192	192	# Recreate the CasBasedDirectory object from the cached artifact
...	...	@@ -231,8 +231,8 @@ def _test_push_directory(user_config_file, project_dir, artifact_dir, artifact_d
231	231	cas = context.artifactcache
232	232
233	233	# Manually setup the CAS remote
234		- cas.setup_remotes(use_config=True)
235		- cas.initialize_remotes()
	234	+ remotes = cas.get_remotes_from_projects()
	235	+ cas.setup_remotes(remotes=remotes)
236	236
237	237	if cas.has_push_remotes():
238	238	# Create a CasBasedDirectory from local CAS cache content
...	...	@@ -307,8 +307,8 @@ def _test_push_message(user_config_file, project_dir, artifact_dir, queue):
307	307	cas = context.artifactcache
308	308
309	309	# Manually setup the CAS remote
310		- cas.setup_remotes(use_config=True)
311		- cas.initialize_remotes()
	310	+ remotes = cas.get_remotes_from_projects()
	311	+ cas.setup_remotes(remotes=remotes)
312	312
313	313	if cas.has_push_remotes():
314	314	# Create an example message 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.