Open BuildStream-Migration-Bot opened 3 years ago
In GitLab by [Gitlab user @tristanvb] on Nov 5, 2020, 07:35
This is worth a quick review of the artifact cache code, should be fairly easy either check that none of this is happening anymore or to simply fix these, I suspect they are mostly fixed by now.
See original issue on GitLab In GitLab by [Gitlab user @tristanvb] on May 6, 2019, 06:03
While building, I am getting errors as such reported:
A missing artifact is actually a bug, not an error; the user cannot fix this, and it is not a system error, it is clearly our fault if this error is ever seen.
Currently we have a contorted story around these errors, and we mitigate this by calling
Element.__assert_cached()
in some places, but not all places.I think this is backwards and unsafe, we should reverse this such that the underlying
CAS
errors do not inherit fromBstError
, and haveArtifactCache
only handle the recoverable errors and turn them into aBstError
derivingArtifactError
.In bst-1.2
When staging a built workspace with a missing artifact (after replacing a
raise ArtifactError
with araise AssertionError
), we get the following codepath which leads to a bug misreported as an error:In master
Here I have not checked the stack trace yet, but I have verified that the code still runs in the same fashion: We raise a
CASCacheError
which derives fromBstError
, and we sprinkleself.__assert_cached()
statements aroundelement.py
in the hopes of covering any case where we're about to access an artifact which doesnt exist, instead of having the underlyingCASCacheError
be treated as a simple exception, and reporting the recoverable errors asArtifactErrors
.In this codepath, we would hit the following:
Element.stage_dependency_artifacts()
Artifact
for the old (missing) workspaced artifactArtifact.get_metadata_dependencies()
Artifact.get_artifact_directory()
ArtifactError
, which will inform the user that the artifact is not available, instead of reporting a bug with a stack trace.Instead, we should have a
CASCacheError
which is not aBstError
, and we should simply notexcept CASCacheError
in that case, ensuring that the missing artifact is reported as aBUG
.