spin icon indicating copy to clipboard operation
spin copied to clipboard

Allow modular manifest files

Open itowlson opened this issue 1 year ago • 2 comments

Fixes #1759.

This is WIP for discussion and pondering.

The proposed UX for the modular feature is to overload the trigger.component field to allow it to reference a file (or directory containing a well-known file). I considered an embedded table for this ([[trigger.http]] component = { path = "example.toml" }) but it felt wordy, so the current draft instead proposes an @filename syntax similar to what we use for the --sqlite flag ([[trigger.http]] component = "@example.toml"). I kind of like it, but suspect it will be divisive!

There are also a couple of serious structural considerations:

  1. The duplication of manifest resolution into the build loader, mentioned in #2393, increases yet more, which is extremely not good. I don't have a good solution for this - I'm toying with some ideas that could defer the full parse in cases where it's not needed, but nothing looks super easy.

  2. This implementation fails on the requirement of tracing errors back to the source file path. Core manifest validation will get the file path, but if, for example, the Wasm file mentioned in the child file doesn't exist, it gives you no clue as to which file the error lies in. We do carry some application origin info into the trigger: I'm wondering if we could attach an optional origin field to each component, but we'd not want that going into the lockfile because the lockfile also serves as the OCI thing.

Also, no, there are (almost) no tests yet.

itowlson avatar Mar 25 '24 21:03 itowlson

As anticipated, "@example.toml" does in fact give me feelings :slightly_smiling_face:. While @file is a pretty common convention in CLI args, I personally prefer more explicitness where more structured typing is available. It is more verbose, but - I think - also easier to read. :shrug:

lann avatar Mar 28 '24 13:03 lann

This seems sufficiently design-laden to be worth a SIP.

lann avatar Mar 28 '24 13:03 lann