juergbi commented on code in PR #2147:
URL: https://github.com/apache/buildstream/pull/2147#discussion_r4046232097
##########
src/buildstream/_loader/loader.py:
##########
@@ -251,10 +255,61 @@ def foreach_parent(parent):
for parent in self._alternative_parents:
yield from foreach_parent(parent)
+ # temporary_modified_element()
+ #
+ # Temporarily modify an element by loading the element and applying
modify_elements_function to make the modifications
+ #
+ #
+ # Args:
+ # target (str): The element-path relative bst file
+ # modify_element_function (Callable[[CommentedMap],None]): A function
to modify a given CommentedMap
+ #
+ @contextmanager
+ def temporary_modified_element(
+ self, target: str, modify_element_function: Callable[[CommentedMap],
None]
+ ) -> Generator[None, None, None]:
+
+ _, target_name, target_loader = self._parse_name(target,
MappingNode.from_dict({}))
+
+ target_path = os.path.join(target_loader._basedir, target_name)
+ target_node: CommentedMap = _yaml.roundtrip_load(target_path)
+
+ modify_element_function(target_node)
+
+ # FIXME When 3.12 hits EOL, replace this with
tempfile.NamedTemporaryFile itself.
+ with _legacy_named_temporary_file_delete_on_close(
Review Comment:
We generally use `utils._tempnamedfile()`, which adds cleanup on SIGTERM.
And we just `.flush()` the temp file without closing it, which works also
without `delete_on_close=False` support.
While moving the utility function to `delete_on_close=False` sounds
reasonable once we require Python 3.12, I don't think it makes sense to add a
helper to the `Loader` here. Let's be consistent and use the same approach as
elsewhere in the codebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]