hundreds-breakfast-49010
01/22/2021, 12:08 AMEngineAware.artifacts
supposed to return a dict of `Digest`s or Union[Snapshot, Digest]
, or some other type?Digest
type worked to support thathundreds-father-404
01/22/2021, 12:11 AMFileDigest
vs. Digest
(directory digest)hundreds-breakfast-49010
01/22/2021, 12:17 AMEngineAwareReturnType.artifacts
Enriched*
workunit)pub stdout: Option<hashing::Digest>,
pub stderr: Option<hashing::Digest>,
WorkunitMetadata
hundreds-father-404
01/22/2021, 12:20 AMright now we handle putting the stdout/stderr output on the workunit for the process execution (which doesn't get created if that process execution is cached, hence the problem)Ah ha. So we need to stop caring about the
Process
workunit, and care purely about the enriched_foo
workunitmessage()
that gets printed to the console, right? For example, via artifacts
Asking because we add some of our own message to what gets printed, e.g. a check or minus signhundreds-breakfast-49010
01/22/2021, 12:23 AMhundreds-father-404
01/22/2021, 12:24 AMSnapshot
always refers to the file system. Are you trying to add something to represent the stdout/stderr? If so, I think you'd want a FileDigest
, which is referring to a str
hundreds-breakfast-49010
01/22/2021, 12:24 AMworkunit_to_py_value
that does:
if let Some(stdout_digest) = &workunit.metadata.stdout.as_ref() {
artifact_entries.push((
externs::store_utf8("stdout_digest"),
crate::nodes::Snapshot::store_file_digest(core, stdout_digest),
));
}
if let Some(stderr_digest) = &workunit.metadata.stderr.as_ref() {
artifact_entries.push((
externs::store_utf8("stderr_digest"),
crate::nodes::Snapshot::store_file_digest(core, stderr_digest),
));
}
Value
on the workunit that has Python type Snapshot
Digest
, I'm not surehundreds-father-404
01/22/2021, 12:26 AMFileDigest
object. The Snapshot
is confusing - that is purely for namespacing of the methodpub fn store_file_digest(core: &Arc<Core>, item: &hashing::Digest) -> Value {
externs::unsafe_call(
core.types.file_digest,
&[
externs::store_utf8(&item.0.to_hex()),
externs::store_i64(item.1 as i64),
],
)
}
core.types.file_digest
hundreds-breakfast-49010
01/22/2021, 12:26 AMFileDigest
Union[Snapshot, FileDigest]
Snapshot::store_file_digest
for handling stdout/stderr in this exact caseEngineAwareReturnType.artifacts
fromdef artifacts(self) -> Optional[Dict[str, Snapshot]]:
to
def artifacts(self) -> Optional[Dict[str, Union[Snapshot, FileDigest]]]:
hundreds-father-404
01/22/2021, 12:29 AMso I think this implies that the right type schema for artifacts is Union[Snapshot, FileDigest]Yep. Although, I'm not sure we should still use
Snapshot
. It could be Digest
.
At the time, there was no FileDigest
so it was ambiguous what was meant by Digest
, hence us wanting to use the unambiguous Snapshot
to refer to the file systemUnion[Digest, FileDigest]
?
Snapshot
may be more ergonomic for consumers, I'm not sure how it gets consumedhundreds-breakfast-49010
01/22/2021, 12:33 AMDigests
hundreds-father-404
01/22/2021, 12:37 AMhundreds-breakfast-49010
01/22/2021, 12:37 AMartifacts
or even just passing non-Snapshot
values in that method regardless of the type annotations breaks things at the engine level in a way that I spent way too long trying to untangle