gtristan commented on a change in pull request #1450:
URL: https://github.com/apache/buildstream/pull/1450#discussion_r586190296
##########
File path: src/buildstream/_artifactelement.py
##########
@@ -131,9 +131,9 @@ def clear_artifact_name_cache(cls):
# Once we've finished pulling an artifact, we assume the
# state of the pulled artifact.
#
- def _pull_done(self):
- super()._pull_done()
+ def _load_artifact_done(self):
Review comment:
I think we need to streamline how `ArtifactElement` instances are
initialized with this.
This branch changes a bit the semantics of how an artifact is loaded onto an
element, but does not appear to update `Element.__initialize_from_artifact()`,
perhaps we can have `Element.__initialize_from_artifact()` directly call
`Element._load_artifact()` or such.
Another point, is that loading the artifact from the local cache is
obviously a hard requirement when loading any `ArtifactElement`, regardless of
whether `Stream.query_cache()` was called on the element.
I think this branch does not exactly change the current initialization (and
deferring the artifact loading is planned in a later branch), in any case, it
should be clear somehow (perhaps in the `Stream.query_cache()` API comments)
that this will end up being a `noop` for `ArtifactElements` which are anyway
synchronously loaded at instantiation time.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
