[Notes] [Git][BuildStream/buildstream][Qinusty/skipped-rework] Add SkipError for indicating a skipped activity

[Qinusty <gitlab mg gitlab com>]

Title: GitLab

Qinusty pushed to branch Qinusty/skipped-rework at BuildStream / buildstream

Commits:

• 5431bce1
  by Josh Smith at 2018-08-30T16:32:14Z

  Add SkipError for indicating a skipped activity
  
  SkipError can now be raised within the context of a timed_activity() to
  trigger a SKIPPED message instead of a FAILURE message.
  
  This changes the previous usage of SKIPPED within messages to use the
  INFO MessageType (used within cascache.py).
  
  Element._pull() now performs its actions from within a timed_activity()
  to match Element._push() and allow for SkipError to be raised.
  
  "Downloaded artifact <cache key>" has been reworded to "Pulled artifact
  <cache key>" for consistency with Element._push().
  

6 changed files:

• buildstream/_artifactcache/cascache.py
• buildstream/_context.py
• buildstream/_exceptions.py
• buildstream/element.py
• tests/frontend/pull.py
• tests/testutils/runcli.py

Changes:

buildstream/_artifactcache/cascache.py

---

...	...	@@ -252,7 +252,7 @@ class CASCache(ArtifactCache):
252	252	else:
253	253	self.context.message(Message(
254	254	None,
255		- MessageType.SKIPPED,
	255	+ MessageType.INFO,
256	256	"Remote ({}) does not have {} cached".format(
257	257	remote.spec.url, element._get_brief_display_key())
258	258	))
...	...	@@ -363,7 +363,7 @@ class CASCache(ArtifactCache):
363	363	if skipped_remote:
364	364	self.context.message(Message(
365	365	None,
366		- MessageType.SKIPPED,
	366	+ MessageType.INFO,
367	367	"Remote ({}) already has {} cached".format(
368	368	remote.spec.url, element._get_brief_display_key())
369	369	))

buildstream/_context.py

---

...	...	@@ -26,7 +26,7 @@ from . import _cachekey
26	26	from . import _signals
27	27	from . import _site
28	28	from . import _yaml
29		-from ._exceptions import LoadError, LoadErrorReason, BstError
	29	+from ._exceptions import LoadError, LoadErrorReason, BstError, SkipError
30	30	from ._message import Message, MessageType
31	31	from ._profile import Topics, profile_start, profile_end
32	32	from ._artifactcache import ArtifactCache
...	...	@@ -406,7 +406,15 @@ class Context():
406	406	self._push_message_depth(silent_nested)
407	407	yield
408	408
409		- except BstError:
	409	+ except SkipError:
	410	+ elapsed = datetime.datetime.now() - starttime
	411	+ message = Message(unique_id, MessageType.SKIPPED, activity_name, elapsed=elapsed)
	412	+ self._pop_message_depth()
	413	+ self.message(message)
	414	+ # Return to the scope outside of the context
	415	+ return
	416	+
	417	+ except BstError as e:
410	418	# Note the failure in status messages and reraise, the scheduler
411	419	# expects an error when there is an error.
412	420	elapsed = datetime.datetime.now() - starttime

buildstream/_exceptions.py

---

...	...	@@ -90,6 +90,7 @@ class ErrorDomain(Enum):
90	90	APP = 12
91	91	STREAM = 13
92	92	VIRTUAL_FS = 14
	93	+ SKIP = 15
93	94
94	95
95	96	# BstError is an internal base exception class for BuildSream
...	...	@@ -309,3 +310,8 @@ class StreamError(BstError):
309	310	class AppError(BstError):
310	311	def __init__(self, message, detail=None, reason=None):
311	312	super().__init__(message, detail=detail, domain=ErrorDomain.APP, reason=reason)
	313	+
	314	+
	315	+class SkipError(BstError):
	316	+ def __init__(self, *, message=None, detail=None, reason=None):
	317	+ super().__init__(message, detail=detail, domain=ErrorDomain.SKIP, reason=reason)

buildstream/element.py

---

...	...	@@ -84,7 +84,7 @@ import shutil
84	84	from . import _yaml
85	85	from ._variables import Variables
86	86	from ._versions import BST_CORE_ARTIFACT_VERSION
87		-from ._exceptions import BstError, LoadError, LoadErrorReason, ImplError, ErrorDomain
	87	+from ._exceptions import BstError, LoadError, LoadErrorReason, ImplError, ErrorDomain, SkipError
88	88	from .utils import UtilError
89	89	from . import Plugin, Consistency
90	90	from . import SandboxFlags
...	...	@@ -1735,19 +1735,20 @@ class Element(Plugin):
1735	1735	def progress(percent, message):
1736	1736	self.status(message)
1737	1737
1738		- # Attempt to pull artifact without knowing whether it's available
1739		- pulled = self._pull_strong(progress=progress)
	1738	+ display_key = self._get_brief_display_key()
	1739	+ with self.timed_activity("Pulling artifact {}".format(display_key)):
	1740	+ # Attempt to pull artifact without knowing whether it's available
	1741	+ pulled = self._pull_strong(progress=progress)
1740	1742
1741		- if not pulled and not self._cached() and not context.get_strict():
1742		- pulled = self._pull_weak(progress=progress)
	1743	+ if not pulled and not self._cached() and not context.get_strict():
	1744	+ pulled = self._pull_weak(progress=progress)
1743	1745
1744		- if not pulled:
1745		- return False
	1746	+ if not pulled:
	1747	+ raise SkipError()
1746	1748
1747		- # Notify successfull download
1748		- display_key = self._get_brief_display_key()
1749		- self.info("Downloaded artifact {}".format(display_key))
1750		- return True
	1749	+ # Notify successfull download
	1750	+ self.info("Pulled artifact {}".format(display_key))
	1751	+ return pulled
1751	1752
1752	1753	# _skip_push():
1753	1754	#
...	...	@@ -1785,16 +1786,17 @@ class Element(Plugin):
1785	1786	self.warn("Not pushing tainted artifact.")
1786	1787	return False
1787	1788
	1789	+ pushed = False
1788	1790	display_key = self._get_brief_display_key()
1789	1791	with self.timed_activity("Pushing artifact {}".format(display_key)):
1790	1792	# Push all keys used for local commit
1791	1793	pushed = self.__artifacts.push(self, self.__get_cache_keys_for_commit())
1792	1794	if not pushed:
1793		- return False
	1795	+ raise SkipError()
1794	1796
1795	1797	# Notify successful upload
1796	1798	self.info("Pushed artifact {}".format(display_key))
1797		- return True
	1799	+ return pushed
1798	1800
1799	1801	# _shell():
1800	1802	#

tests/frontend/pull.py

---

...	...	@@ -356,4 +356,5 @@ def test_pull_missing_notifies_user(caplog, cli, tmpdir, datafiles):
356	356	assert not result.get_pulled_elements(), \
357	357	"No elements should have been pulled since the cache was empty"
358	358
359		- assert "SKIPPED Remote ({}) does not have".format(share.repo) in result.stderr
	359	+ assert "INFO Remote ({}) does not have".format(share.repo) in result.stderr
	360	+ assert "SKIPPED Pulling artifact" in result.stderr

tests/testutils/runcli.py

---

...	...	@@ -178,7 +178,7 @@ class Result():
178	178	return list(pushed)
179	179
180	180	def get_pulled_elements(self):
181		- pulled = re.findall(r'\[\s*pull:(\S+)\s*\]\s*INFO\s*Downloaded artifact', self.stderr)
	181	+ pulled = re.findall(r'\[\s*pull:(\S+)\s*\]\s*INFO\s*Pulled artifact', self.stderr)
182	182	if pulled is None:
183	183	return []
184	184

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