-
Notifications
You must be signed in to change notification settings - Fork 249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spin Build Targets Check SIP #2556
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Brian H <[email protected]>
``` | ||
|
||
# Open question: How does this work with custom triggers? | ||
One possible solution is to allow trigger extensions the abilitiy to hook into build time validation via the `TriggerExecutor` trait. For each `spin build`, the `spin-cli` could construct a `build` command to invoke each trigger with allowing the custom trigger an opportunity to enforce validation on the target of each component before `up`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Could add a standard e.g. --trigger-validate-app
flag.
Somewhat of a tangent, but we might want to think about how to avoid future collisions with trigger-specific flags; maybe we should reserve e.g. --trigger-*
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, --trigger-validate-app
(or something) is a better idea. Also +1 to reserving --trigger-*
flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure asking the trigger plugin is a good direction to go. It will solve the immediate "does my component match my trigger" question but that's not the real problem we're trying to lay foundations for. To refer back to the long game, suppose a user is running Spin 4.0 with v3.5 of the cron trigger, but specifies they want to the app to run on Spn 3.31 with cron 2.2, and on SpinKube 3.2 (with whatever cron trigger that includes). Asking the local trigger is not going to help us there. Maybe I'm YAGNI-ing though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative @lann idea is to be able to query custom triggers for the worlds they support..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@itowlson I'm not sure I follow your argument entirely.
Let me state where I'm coming from with the feature and see whether we're on the same page here.
The goal of the feature is to determine at build time whether a certain component conforms to a given versioned world; in your counter-example, if the user specifies a version of the world that the local trigger plugin configured does not support, I would expect the check to fail (and ideally, there would be an error that plugins should be updated)?
If is not the responsibility of trigger plugins, then I'm not sure how else you would be able to validate that, given Spin itself would have no knowledge of arbitrary worlds (outside of say HTTP and whatever else might be "default").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(of course I should have just read the thread until the end and would have seen your detailed comment below...)
Component "hello" is not a valid http component. | ||
|
||
Error: | ||
Expected component to either export `wasi:http/[email protected]` or `fermyon:spin/inbound-http` but it exported neither |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing this specific example brings up is that we probably don't one one monolithic "make sure it implements world W or bail" since the http trigger accepts several different worlds. I'm guessing a more flexible "validate this app" feature would work well here, combined with a library that makes it easy to check for targeting individual worlds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some pedantry: the http trigger accepts several different guest worlds, but it implements one world (the http trigger world). In other worlds, the host world is a union of the various guest worlds it accepts. One way to think about this issue is: what world does the guest implement, and is that world a sub-type (sub-world?) of the host world?
I'm not sure this has an impact on how we're thinking about this feature in this SIP, but it might have an impact on implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess in the terminology of this SIP, "HTTP apps target at least one of several worlds."
I was thinking a bit more about the custom triggers thing, and wanted to take a moment to align our understandings. For me, the main value of this work is kind of a technology proof for validating against a world once we have a world (or, more broadly, a set of acceptable worlds). That's a prerequisite for target environments, but there's a whole lot of additional work in defining what an environment is and how we understand it. Again consider the "I want to target SpinKube 3.2" scenario. SpinKube 3.2 isn't a world: it's a mapping of trigger types to worlds. We will need to understand from some source what that mapping is. The custom triggers discussion seems to cross over into the "understand the environment / mapping" problem, but in a way that is very specific to the "the version of Spin I am running here and now" environment. Yes, we want to be able to validate for that local environment for sure. But I'm wondering if we should punt on solving the mapping problem for that "local Spin" environment until we can consider it alongside other environments. As I say, that's just my envisioned purpose and scope for this leg of the work. It may be (it seems likely) that other folks have a different vision and scope, so I'd like to crisp up what that is and how it fits into the eventual goal. |
I would be fine with us making the decision to hold off on solving this for custom triggers (for now) until we firm up our longer term goals (those of which are still not entirely clear to me at the moment). |
I would also be aligned to solving this for the set of known worlds and not tackle custom triggers and worlds in the first go. |
@radu-matei Thanks. I have a technology POC that uses the Now, we can and will invest in those upstream projects to improve this - but at the moment, @fibonacci1729 is on point for that, and he is also trying to land component dependencies in Spin. Given that, and that component dependencies will affect how target validation works, the current proposal was for him to prioritise component dependencies, then resume the upstream and target worlds stuff. @fibonacci1729 hope this is an accurate representation of the current thinking - please correct me if not! |
Rendered