Open Mossaka opened 10 months ago
I wouldn't want to see these in spin-app
, which relates specifically to the runtime environment and is bound to spin-core
and wasmtime
, but agree they may be reusable (cloud-plugin
has very similar logic and has the same goal of loading interchangeably from spin.toml
or OCI reference). They are bound up with the load process and particularly with how the Spin UI resolves references (whether paths or OCI references), so maybe in loader, or in their own crate so as to be specific and limited about dependencies...?
Yeah that makes sense. I am fine with either solutions as long as they are shareable in some form because we can't depend on spin-cli
.
Maybe we can create a new crate like spin-cli-lib
and move as much logic from spin cli to it as possible. WDYT?
My feelings about that are very mixed.
On the one hand, yes, it could be a powerful enabler of consistent behaviour and experience across different Spin runtime hosts (for hosts that want to opt into Spin CLI-like experiences, of course).
On the other hand, it likely ends up linking to everything in the world, meaning that innocent helper tools that just want to resolve a --from
flag end up dragging in the whole of the runtime (as cloud-plugin did until recently!). This can technically be avoided with features, but features are unreliable at avoiding unintended dependencies. But perhaps breaking out lighter-weight subsets could be deferred - the cli-lib crate could always re-export broken-out types just as spin-app now does.
Let's have a look and see if any existing crates feel like a good home for these types. Otherwise, I'd suggest creating a tiny new crate for them, with cli-lib as a longer term idea. Thoughts?
For the time being, I think it would be great to add them to the loader crate. I can do some work here.
There isn't very much code in (Resolved)AppSource
; could you clarify which part(s) of it would be useful to share?
I was hoping to refactor this section of the code out: https://github.com/deislabs/containerd-wasm-shims/blob/main/containerd-shim-spin/src/engine.rs#L266-L294
ResolvedAppSource
exists for spin up
to be able to "partially load" an app just to make trigger-specific --help
work properly. Since the shim (presumably) doesn't need to do that I would suggest skipping that step and going straight to the LockedApp
. Then you only need the OciRegistry arm of that function: https://github.com/deislabs/containerd-wasm-shims/blob/81d24438921494684ea9aaf0c3573dde01bf20df/containerd-shim-spin/src/engine.rs#L283-L292
As a maintainer of the spin shim, I found these two structs particularly useful for running spin application either from the local filesystem or from oci, but we can't directly take a dependency on the
spin-cli
crate due tobuild.rs
. In order to share more logic between theup
command and the spinEngine
implementation in the containerd-shim, I'd suggest to move these two structs tospin_app
crate that we depend on. Thoughts?See: https://github.com/deislabs/containerd-wasm-shims/pull/164