#Checks TUI
1 messages ยท Page 1 of 1 (latest)
To get this to work I made a change to how checks work: now you can return an EnvironmentCheck instead of a Result, which will be evaluated recursively. I had to do this so that it gets evaluated top-down instead of bottom-up, otherwise the nested parent-child relationships don't work.
Ah okay, good to know. Did you make the change so that it has to be an EnvironmentCheck or did you just handle both the case where it's an EnvironmentCheck and an EnvironmentCheckResult?
Either is totally fine, just curious because pretty much all of these different entrypoint types have ended up with slightly different behaviors/requirements in terms of what the SDKs are expected to return.
Has been very useful to explore the whole landscape of possibilities, but resulted in tons of kludge everywhere, so one of the followups as part of starting to merge all this stuff w/ clean implementations is going to be to try to find a more cohesive+simple model rather than special handling all over the place
It handles both, in a horrific way. ๐ https://github.com/vito/dagger/commit/1b1b03de54dad2f09cd0e49e4dce0ffd4d33cb29#diff-7df1d347190aa8a61c81127d04d38041f0a0fdcbce86b50503aeee3f12a950eeR655-R673
And yeah, it's honestly kind of fun defining what each type means for each entrypoint. Feels sort of like building out a little language. Just have to make sure it doesn't feel like total hackery in the end (i.e. internally consistent, never a time where you want two different behaviors from one return type, etc).
Yeah I agree, there's some interesting tradeoffs here. On one hand, it's great for user code to be flexible and intuitive (i.e. a Check should be implementable as just returning (string, error), returning just error, returning a CheckResult, returning a Check, etc.), but that means either:
- Every SDK needs to have special handling for this, which is
(work * numSDKs)and also could lead to confusing divergence - The graphql server needs to handle the polymorphism, which is less work for SDKs but creates incredibly convoluted code in the server and will require pretty thorough SDK-developer docs on the exact contract
I think we'll definitely want to try going with 2 and just figure out ways of structuring our code such that it's clear and clean
Agree, I have some loose ideas for 2, like using something like {"type":"Container","id":"eyJ..."} so we can distinguish ID types. Or potentially changing ID to include the type.
Ha, I actually made that exact change of including the type in the serialized ID for Artifacts, but it started devolving so I backed out and just had the SDK handle the polymorphism for now: https://github.com/sipsma/dagger/blob/183e1b22c23de3b36d3ec337bf6201e0ff81b5f8/sdk/go/serveArtifact.go#L118-L118
changing IDs to something like Container:eyJ... could be slightly helpful for debugging too ๐ค
i vaguely remember there being some kind of standard for this...
I guess in theory we could even go so far as to swap out using json for everything with any number of other serialization formats that handle this
Like protobuf / capnproto? I've always been curious about the latter, and it hit 1.0 recently, so checking it out in case it has a nicer toolchain
(not that we need to solve this now)
I went full Protobuf for all of Bass's types, which was ... interesting: https://github.com/vito/bass/blob/main/proto/bass.proto