[Notes] [Git][BuildStream/buildstream][aevri/include-error] 10 commits: plugins/sources: refactor, rm needless ExitStack-s

[Angelos Evripiotis <gitlab mg gitlab com>]

Title: GitLab

Angelos Evripiotis pushed to branch aevri/include-error at BuildStream / buildstream

Commits:

• 74c115b9
  by Angelos Evripiotis at 2018-10-23T10:07:31Z

  plugins/sources: refactor, rm needless ExitStack-s
  
  Remove two uses of ExitStack that aren't necessary and make the code
  a little harder to follow.
  

• ecb58b42
  by Phil Dawson at 2018-10-23T10:33:47Z

  Merge branch 'aevri/rm-exitstack' into 'master'
  
  plugins/sources: refactor, rm needless ExitStack-s
  
  See merge request BuildStream/buildstream!888

• aa0cbf5d
  by Martin Blanchard at 2018-10-23T10:54:40Z

  _sandboxremote.py: Try to reopen operation steam on failure
  
  The REAPI allows a client to reconnect to an ongoing operation stream by
  providing a WaitExecution(). If implemented on server side, BuildStream
  will try to recover from connection errors using it.
  
  https://gitlab.com/BuildStream/buildstream/issues/630
  

• 552f5fc6
  by Jim MacArthur at 2018-10-23T11:19:48Z

  Merge branch 'mablanch/630-remote-execution-reconn' into 'master'
  
  Handle connection losses during remote build execution
  
  See merge request BuildStream/buildstream!806

• 276d2195
  by Angelos Evripiotis at 2018-10-25T10:17:49Z

  _downloadablefilesource: handle ValueError-s
  

• 2d012a21
  by Angelos Evripiotis at 2018-10-25T10:17:49Z

  Merge branch 'aevri/unknown-uri-452' into 'master'
  
  _downloadablefilesource: handle ValueError-s
  
  Closes #452
  
  See merge request BuildStream/buildstream!893

• a9f94a74
  by Angelos Evripiotis at 2018-10-25T13:30:40Z

  _includes: better provenance on recursive include
  
  Use the provenance of the include block, instead of the whole node.
  

• 8f86cd31
  by Angelos Evripiotis at 2018-10-25T13:30:40Z

  _includes: better error on missing include
  
  Previously, a missing include would result in an error like this:
  
      Could not find file at not-a-file.include
  
  Note that the file containing the include was not mentioned.
  
  Now we get an error like this instead:
  
      element.bst [line 7 column 5]: Include block references a file that
      could not be found: 'not-a-file.include'.
  

• 0cea62bc
  by Angelos Evripiotis at 2018-10-25T13:30:40Z

  _includes: better error on including directory
  
  Previously, a missing include would result in an error like this:
  
      mydir is a directory. bst command expects a .bst file.
  
  Note that the file containing the include was not mentioned.
  
  Now we get an error like this instead:
  
      element.bst [line 12 column 0]: Include block references a
      directory instead of a file: 'mydir'.
  

• 7480ad45
  by Angelos Evripiotis at 2018-10-25T13:30:40Z

  _includes: re-use file_path variable
  
  Avoid an unnecessary call to os.path.join().
  

5 changed files:

• buildstream/_includes.py
• buildstream/plugins/sources/_downloadablefilesource.py
• buildstream/plugins/sources/deb.py
• buildstream/plugins/sources/tar.py
• buildstream/sandbox/_sandboxremote.py

Changes:

buildstream/_includes.py

---

...	...	@@ -40,19 +40,34 @@ class Includes:
40	40	includes = [_yaml.node_get(node, str, '(@)')]
41	41	else:
42	42	includes = _yaml.node_get(node, list, '(@)', default_value=None)
	43	+
	44	+ include_provenance = None
43	45	if '(@)' in node:
	46	+ include_provenance = _yaml.node_get_provenance(node, key='(@)')
44	47	del node['(@)']
45	48
46	49	if includes:
47	50	for include in reversed(includes):
48	51	if only_local and ':' in include:
49	52	continue
50		- include_node, file_path, sub_loader = self._include_file(include,
51		- current_loader)
	53	+ try:
	54	+ include_node, file_path, sub_loader = self._include_file(include,
	55	+ current_loader)
	56	+ except LoadError as e:
	57	+ if e.reason == LoadErrorReason.MISSING_FILE:
	58	+ message = "{}: Include block references a file that could not be found: '{}'.".format(
	59	+ include_provenance, include)
	60	+ raise LoadError(LoadErrorReason.MISSING_FILE, message) from e
	61	+ elif e.reason == LoadErrorReason.LOADING_DIRECTORY:
	62	+ message = "{}: Include block references a directory instead of a file: '{}'.".format(
	63	+ include_provenance, include)
	64	+ raise LoadError(LoadErrorReason.LOADING_DIRECTORY, message) from e
	65	+ else:
	66	+ raise
	67	+
52	68	if file_path in included:
53		- provenance = _yaml.node_get_provenance(node)
54	69	raise LoadError(LoadErrorReason.RECURSIVE_INCLUDE,
55		- "{}: trying to recursively include {}". format(provenance,
	70	+ "{}: trying to recursively include {}". format(include_provenance,
56	71	file_path))
57	72	# Because the included node will be modified, we need
58	73	# to copy it so that we do not modify the toplevel
...	...	@@ -101,7 +116,7 @@ class Includes:
101	116	file_path = os.path.join(directory, include)
102	117	key = (current_loader, file_path)
103	118	if key not in self._loaded:
104		- self._loaded[key] = _yaml.load(os.path.join(directory, include),
	119	+ self._loaded[key] = _yaml.load(file_path,
105	120	shortname=shortname,
106	121	project=project,
107	122	copy_tree=self._copy_tree)

buildstream/plugins/sources/_downloadablefilesource.py

---

...	...	@@ -152,7 +152,9 @@ class DownloadableFileSource(Source):
152	152	raise SourceError("{}: Error mirroring {}: {}"
153	153	.format(self, self.url, e), temporary=True) from e
154	154
155		- except (urllib.error.URLError, urllib.error.ContentTooShortError, OSError) as e:
	155	+ except (urllib.error.URLError, urllib.error.ContentTooShortError, OSError, ValueError) as e:
	156	+ # Note that urllib.request.Request in the try block may throw a
	157	+ # ValueError for unknown url types, so we handle it here.
156	158	raise SourceError("{}: Error mirroring {}: {}"
157	159	.format(self, self.url, e), temporary=True) from e
158	160

buildstream/plugins/sources/deb.py

---

...	...	@@ -50,7 +50,7 @@ deb - stage files from .deb packages
50	50	"""
51	51
52	52	import tarfile
53		-from contextlib import contextmanager, ExitStack
	53	+from contextlib import contextmanager
54	54	import arpy # pylint: disable=import-error
55	55
56	56	from .tar import TarSource
...	...	@@ -69,8 +69,7 @@ class DebSource(TarSource):
69	69
70	70	@contextmanager
71	71	def _get_tar(self):
72		- with ExitStack() as context:
73		- deb_file = context.enter_context(open(self._get_mirror_file(), 'rb'))
	72	+ with open(self._get_mirror_file(), 'rb') as deb_file:
74	73	arpy_archive = arpy.Archive(fileobj=deb_file)
75	74	arpy_archive.read_all_headers()
76	75	data_tar_arpy = [v for k, v in arpy_archive.archived_files.items() if b"data.tar" in k][0]

buildstream/plugins/sources/tar.py

---

...	...	@@ -57,7 +57,7 @@ tar - stage files from tar archives
57	57
58	58	import os
59	59	import tarfile
60		-from contextlib import contextmanager, ExitStack
	60	+from contextlib import contextmanager
61	61	from tempfile import TemporaryFile
62	62
63	63	from buildstream import SourceError
...	...	@@ -88,8 +88,7 @@ class TarSource(DownloadableFileSource):
88	88	def _run_lzip(self):
89	89	assert self.host_lzip
90	90	with TemporaryFile() as lzip_stdout:
91		- with ExitStack() as context:
92		- lzip_file = context.enter_context(open(self._get_mirror_file(), 'r'))
	91	+ with open(self._get_mirror_file(), 'r') as lzip_file:
93	92	self.call([self.host_lzip, '-d'],
94	93	stdin=lzip_file,
95	94	stdout=lzip_stdout)

buildstream/sandbox/_sandboxremote.py

---

...	...	@@ -76,8 +76,7 @@ class SandboxRemote(Sandbox):
76	76	# Upload the Command message to the remote CAS server
77	77	command_digest = cascache.push_message(self._get_project(), remote_command)
78	78	if not command_digest or not cascache.verify_digest_pushed(self._get_project(), command_digest):
79		- # Command push failed
80		- return None
	79	+ raise SandboxError("Failed pushing build command to remote CAS.")
81	80
82	81	# Create and send the action.
83	82	action = remote_execution_pb2.Action(command_digest=command_digest,
...	...	@@ -88,27 +87,57 @@ class SandboxRemote(Sandbox):
88	87	# Upload the Action message to the remote CAS server
89	88	action_digest = cascache.push_message(self._get_project(), action)
90	89	if not action_digest or not cascache.verify_digest_pushed(self._get_project(), action_digest):
91		- # Action push failed
92		- return None
	90	+ raise SandboxError("Failed pushing build action to remote CAS.")
93	91
94	92	# Next, try to create a communication channel to the BuildGrid server.
95	93	channel = grpc.insecure_channel(self.server_url)
96	94	stub = remote_execution_pb2_grpc.ExecutionStub(channel)
97	95	request = remote_execution_pb2.ExecuteRequest(action_digest=action_digest,
98	96	skip_cache_lookup=False)
99		- try:
100		- operation_iterator = stub.Execute(request)
101		- except grpc.RpcError:
102		- return None
	97	+
	98	+ def __run_remote_command(stub, execute_request=None, running_operation=None):
	99	+ try:
	100	+ last_operation = None
	101	+ if execute_request is not None:
	102	+ operation_iterator = stub.Execute(execute_request)
	103	+ else:
	104	+ request = remote_execution_pb2.WaitExecutionRequest(name=running_operation.name)
	105	+ operation_iterator = stub.WaitExecution(request)
	106	+
	107	+ for operation in operation_iterator:
	108	+ if operation.done:
	109	+ return operation
	110	+ else:
	111	+ last_operation = operation
	112	+ except grpc.RpcError as e:
	113	+ status_code = e.code()
	114	+ if status_code == grpc.StatusCode.UNAVAILABLE:
	115	+ raise SandboxError("Failed contacting remote execution server at {}."
	116	+ .format(self.server_url))
	117	+
	118	+ elif status_code in (grpc.StatusCode.INVALID_ARGUMENT,
	119	+ grpc.StatusCode.FAILED_PRECONDITION,
	120	+ grpc.StatusCode.RESOURCE_EXHAUSTED,
	121	+ grpc.StatusCode.INTERNAL,
	122	+ grpc.StatusCode.DEADLINE_EXCEEDED):
	123	+ raise SandboxError("{} ({}).".format(e.details(), status_code.name))
	124	+
	125	+ elif running_operation and status_code == grpc.StatusCode.UNIMPLEMENTED:
	126	+ raise SandboxError("Failed trying to recover from connection loss: "
	127	+ "server does not support operation status polling recovery.")
	128	+
	129	+ return last_operation
103	130
104	131	operation = None
105	132	with self._get_context().timed_activity("Waiting for the remote build to complete"):
106		- # It is advantageous to check operation_iterator.code() is grpc.StatusCode.OK here,
107		- # which will check the server is actually contactable. However, calling it when the
108		- # server is available seems to cause .code() to hang forever.
109		- for operation in operation_iterator:
110		- if operation.done:
111		- break
	133	+ operation = __run_remote_command(stub, execute_request=request)
	134	+ if operation is None:
	135	+ return None
	136	+ elif operation.done:
	137	+ return operation
	138	+
	139	+ while operation is not None and not operation.done:
	140	+ operation = __run_remote_command(stub, running_operation=operation)
112	141
113	142	return operation
114	143
...	...	@@ -192,7 +221,6 @@ class SandboxRemote(Sandbox):
192	221
193	222	if operation is None:
194	223	# Failure of remote execution, usually due to an error in BuildStream
195		- # NB This error could be raised in __run_remote_command
196	224	raise SandboxError("No response returned from server")
197	225
198	226	assert not operation.HasField('error') and operation.HasField('response')

—
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.