[Notes] [Git][BuildStream/buildstream][lachlan/pickle-yaml-test-list-composite] SQUASHME: yamlcache: hide key calculation and fix public/private

[Jonathan Maw <gitlab mg gitlab com>]

Title: GitLab

Jonathan Maw pushed to branch lachlan/pickle-yaml-test-list-composite at BuildStream / buildstream

Commits:

• 436cf9c3
  by Jonathan Maw at 2018-10-08T16:27:40Z

  SQUASHME: yamlcache: hide key calculation and fix public/private
  
  Previously, key calculation had to be done manually, creating
  unnecessary boilerplate. This has now been subsumed into YamlCache.get
  and YamlCache.put (Yamlcache._get and Yamlcache._put_from_key exist for
  old behaviour).
  
  YamlCache.write is now private.
  
  Private methods have been put at the end of the class.
  

3 changed files:

• buildstream/_yaml.py
• buildstream/_yamlcache.py
• tests/frontend/yamlcache.py

Changes:

buildstream/_yaml.py

---

...	...	@@ -200,14 +200,13 @@ def load(filename, shortname=None, copy_tree=False, *, project=None, yaml_cache=
200	200	with open(filename) as f:
201	201	contents = f.read()
202	202	if yaml_cache:
203		- key = yaml_cache.calculate_key(contents, copy_tree)
204		- data = yaml_cache.get(project, filename, key)
	203	+ data, key = yaml_cache.get(project, filename, contents, copy_tree)
205	204
206	205	if not data:
207	206	data = load_data(contents, file, copy_tree=copy_tree)
208	207
209	208	if yaml_cache:
210		- yaml_cache.put(project, filename, key, data)
	209	+ yaml_cache.put_from_key(project, filename, key, data)
211	210
212	211	return data
213	212	except FileNotFoundError as e:

buildstream/_yamlcache.py

---

...	...	@@ -52,72 +52,20 @@ class YamlCache():
52	52	def __init__(self, context):
53	53	self._project_caches = {}
54	54	self._context = context
	55	+ ##################
	56	+ # Public Methods #
	57	+ ##################
55	58
56		- # Writes the yaml cache to the specified path.
	59	+ # is_cached():
57	60	#
58		- # Args:
59		- # path (str): The path to the cache file.
60		- def write(self, path):
61		- parent_dir = os.path.dirname(path)
62		- os.makedirs(parent_dir, exist_ok=True)
63		- with open(path, "wb") as f:
64		- BstPickler(f).dump(self)
65		-
66		- # Gets a parsed file from the cache.
	61	+ # Checks whether a file is cached.
67	62	#
68	63	# Args:
69	64	# project (Project): The project this file is in.
70		- # filepath (str): The path to the file.
71		- # key (str): The key to the file within the cache. Typically, this is the
72		- # value of `calculate_key()` with the file's unparsed contents
73		- # and any relevant metadata passed in.
74		- #
75		- # Returns:
76		- # (decorated dict): The parsed yaml from the cache, or None if the file isn't in the cache.
77		- def get(self, project, filepath, key):
78		- cache_path = self._get_filepath(project, filepath)
79		- project_name = project.name if project else ""
80		- try:
81		- project_cache = self._project_caches[project_name]
82		- try:
83		- cachedyaml = project_cache.elements[cache_path]
84		- if cachedyaml._key == key:
85		- # We've unpickled the YamlCache, but not the specific file
86		- if cachedyaml._contents is None:
87		- cachedyaml._contents = BstUnpickler.loads(cachedyaml._pickled_contents, self._context)
88		- return cachedyaml._contents
89		- except KeyError:
90		- pass
91		- except KeyError:
92		- pass
93		- return None
94		-
95		- # Put a parsed file into the cache.
96		- #
97		- # Args:
98		- # project (Project): The project this file is in.
99		- # filepath (str): The path to the file.
100		- # key (str): The key to the file within the cache. Typically, this is the
101		- # value of `calculate_key()` with the file's unparsed contents
102		- # and any relevant metadata passed in.
103		- # value (decorated dict): The data to put into the cache.
104		- def put(self, project, filepath, key, value):
105		- cache_path = self._get_filepath(project, filepath)
106		- project_name = project.name if project else ""
107		- try:
108		- project_cache = self._project_caches[project_name]
109		- except KeyError:
110		- project_cache = self._project_caches[project_name] = CachedProject({})
111		-
112		- project_cache.elements[cache_path] = CachedYaml(key, value)
113		-
114		- # Checks whether a file is cached
115		- # Args:
116		- # project (Project): The project this file is in.
117	65	# filepath (str): The path to the file, *relative to the project's directory*.
118	66	#
119	67	# Returns:
120		- # (bool): Whether the file is cached
	68	+ # (bool): Whether the file is cached.
121	69	def is_cached(self, project, filepath):
122	70	cache_path = self._get_filepath(project, filepath)
123	71	project_name = project.name if project else ""
...	...	@@ -129,14 +77,8 @@ class YamlCache():
129	77	pass
130	78	return False
131	79
132		- def _get_filepath(self, project, full_path):
133		- if project:
134		- assert full_path.startswith(project.directory)
135		- filepath = os.path.relpath(full_path, project.directory)
136		- else:
137		- filepath = full_path
138		- return full_path
139		-
	80	+ # open():
	81	+ #
140	82	# Return an instance of the YamlCache which writes to disk when it leaves scope.
141	83	#
142	84	# Args:
...	...	@@ -163,18 +105,151 @@ class YamlCache():
163	105
164	106	yield cache
165	107
166		- cache.write(cachefile)
	108	+ cache._write(cachefile)
	109	+
	110	+ # get_cache_file():
	111	+ #
	112	+ # Retrieves a path to the yaml cache file.
	113	+ #
	114	+ # Returns:
	115	+ # (str): The path to the cache file
	116	+ @staticmethod
	117	+ def get_cache_file(top_dir):
	118	+ return os.path.join(top_dir, ".bst", YAML_CACHE_FILENAME)
	119	+
	120	+ # get():
	121	+ #
	122	+ # Gets a parsed file from the cache.
	123	+ #
	124	+ # Args:
	125	+ # project (Project) or None: The project this file is in, if it exists.
	126	+ # filepath (str): The absolute path to the file.
	127	+ # contents (str): The contents of the file to be cached
	128	+ # copy_tree (bool): Whether the data should make a copy when it's being generated
	129	+ # (i.e. exactly as when called in yaml)
	130	+ #
	131	+ # Returns:
	132	+ # (decorated dict): The parsed yaml from the cache, or None if the file isn't in the cache.
	133	+ # (str): The key used to look up the parsed yaml in the cache
	134	+ def get(self, project, filepath, contents, copy_tree):
	135	+ key = self._calculate_key(contents, copy_tree)
	136	+ data = self._get(project, filepath, key)
	137	+ return data, key
	138	+
	139	+ # put():
	140	+ #
	141	+ # Puts a parsed file into the cache.
	142	+ #
	143	+ # Args:
	144	+ # project (Project): The project this file is in.
	145	+ # filepath (str): The path to the file.
	146	+ # contents (str): The contents of the file that has been cached
	147	+ # copy_tree (bool): Whether the data should make a copy when it's being generated
	148	+ # (i.e. exactly as when called in yaml)
	149	+ # value (decorated dict): The data to put into the cache.
	150	+ def put(self, project, filepath, contents, copy_tree, value):
	151	+ key = self._calculate_key(contents, copy_tree)
	152	+ self.put_from_key(project, filepath, key, value)
	153	+
	154	+ # put_from_key():
	155	+ #
	156	+ # Put a parsed file into the cache when given a key.
	157	+ #
	158	+ # Args:
	159	+ # project (Project): The project this file is in.
	160	+ # filepath (str): The path to the file.
	161	+ # key (str): The key to the file within the cache. Typically, this is the
	162	+ # value of `calculate_key()` with the file's unparsed contents
	163	+ # and any relevant metadata passed in.
	164	+ # value (decorated dict): The data to put into the cache.
	165	+ def put_from_key(self, project, filepath, key, value):
	166	+ cache_path = self._get_filepath(project, filepath)
	167	+ project_name = project.name if project else ""
	168	+ try:
	169	+ project_cache = self._project_caches[project_name]
	170	+ except KeyError:
	171	+ project_cache = self._project_caches[project_name] = CachedProject({})
	172	+
	173	+ project_cache.elements[cache_path] = CachedYaml(key, value)
167	174
	175	+
	176	+ ###################
	177	+ # Private Methods #
	178	+ ###################
	179	+
	180	+ # Writes the yaml cache to the specified path.
	181	+ #
	182	+ # Args:
	183	+ # path (str): The path to the cache file.
	184	+ def _write(self, path):
	185	+ parent_dir = os.path.dirname(path)
	186	+ os.makedirs(parent_dir, exist_ok=True)
	187	+ with open(path, "wb") as f:
	188	+ BstPickler(f).dump(self)
	189	+
	190	+ # _get_filepath():
	191	+ #
	192	+ # Returns a file path relative to a project if passed, or the original path if
	193	+ # the project is None
	194	+ #
	195	+ # Args:
	196	+ # project (Project) or None: The project the filepath exists within
	197	+ # full_path (str): The path that the returned path is based on
	198	+ #
	199	+ # Returns:
	200	+ # (str): The path to the file, relative to a project if it exists
	201	+ def _get_filepath(self, project, full_path):
	202	+ if project:
	203	+ assert full_path.startswith(project.directory)
	204	+ filepath = os.path.relpath(full_path, project.directory)
	205	+ else:
	206	+ filepath = full_path
	207	+ return full_path
	208	+
	209	+ # _calculate_key():
	210	+ #
168	211	# Calculates a key for putting into the cache.
	212	+ #
	213	+ # Args:
	214	+ # (basic object)... : Any number of strictly-ordered basic objects
	215	+ #
	216	+ # Returns:
	217	+ # (str): A key made out of every arg passed in
169	218	@staticmethod
170		- def calculate_key(*args):
	219	+ def _calculate_key(*args):
171	220	string = pickle.dumps(args)
172	221	return hashlib.sha1(string).hexdigest()
173	222
174		- # Retrieves a path to the yaml cache file.
175		- @staticmethod
176		- def get_cache_file(top_dir):
177		- return os.path.join(top_dir, ".bst", YAML_CACHE_FILENAME)
	223	+ # _get():
	224	+ #
	225	+ # Gets a parsed file from the cache when given a key.
	226	+ #
	227	+ # Args:
	228	+ # project (Project): The project this file is in.
	229	+ # filepath (str): The path to the file.
	230	+ # key (str): The key to the file within the cache. Typically, this is the
	231	+ # value of `calculate_key()` with the file's unparsed contents
	232	+ # and any relevant metadata passed in.
	233	+ #
	234	+ # Returns:
	235	+ # (decorated dict): The parsed yaml from the cache, or None if the file isn't in the cache.
	236	+ def _get(self, project, filepath, key):
	237	+ cache_path = self._get_filepath(project, filepath)
	238	+ project_name = project.name if project else ""
	239	+ try:
	240	+ project_cache = self._project_caches[project_name]
	241	+ try:
	242	+ cachedyaml = project_cache.elements[cache_path]
	243	+ if cachedyaml._key == key:
	244	+ # We've unpickled the YamlCache, but not the specific file
	245	+ if cachedyaml._contents is None:
	246	+ cachedyaml._contents = BstUnpickler.loads(cachedyaml._pickled_contents, self._context)
	247	+ return cachedyaml._contents
	248	+ except KeyError:
	249	+ pass
	250	+ except KeyError:
	251	+ pass
	252	+ return None
178	253
179	254
180	255	CachedProject = namedtuple('CachedProject', ['elements'])

tests/frontend/yamlcache.py

---

...	...	@@ -59,7 +59,7 @@ def with_yamlcache(project_dir):
59	59
60	60	def yamlcache_key(yamlcache, in_file, copy_tree=False):
61	61	with open(in_file) as f:
62		- key = yamlcache.calculate_key(f.read(), copy_tree)
	62	+ key = yamlcache._calculate_key(f.read(), copy_tree)
63	63	return key
64	64
65	65
...	...	@@ -100,7 +100,7 @@ def test_yamlcache_used(cli, tmpdir, ref_storage, with_junction, move_project):
100	100	temppath = modified_file(element_path, str(tmpdir))
101	101	contents = _yaml.load(temppath, copy_tree=False, project=prj)
102	102	key = yamlcache_key(yc, element_path)
103		- yc.put(prj, element_path, key, contents)
	103	+ yc.put_from_key(prj, element_path, key, contents)
104	104
105	105	# Show that a variable has been added
106	106	result = cli.run(project=project, args=['show', '--format', '%{vars}', 'test.bst'])
...	...	@@ -114,13 +114,6 @@ def test_yamlcache_used(cli, tmpdir, ref_storage, with_junction, move_project):
114	114	@pytest.mark.parametrize('with_junction', ['junction', 'no-junction'])
115	115	@pytest.mark.parametrize('move_project', ['move', 'no-move'])
116	116	def test_yamlcache_changed_file(cli, ref_storage, with_junction, move_project):
	117	+ # i.e. a file is cached, the file is changed, loading the file (with cache) returns new data
117	118	# inline and junction can only be changed by opening a workspace
118	119	pass
119		-
120		-
121		-@pytest.mark.parametrize('ref_storage', ['inline', 'project.refs'])
122		-@pytest.mark.parametrize('with_junction', ['junction', 'no-junction'])
123		-@pytest.mark.parametrize('move_project', ['move', 'no-move'])
124		-def test_yamlcache_track_changed(cli, ref_storage, with_junction, move_project):
125		- # Skip inline junction, tracking those is forbidden
126		- pass

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