Skip to content

WIP: add support for preview 2 components that target the wasi:cli world #1

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

Merged
merged 3 commits into from
Mar 29, 2024

Conversation

karthik2804
Copy link
Contributor

@karthik2804 karthik2804 commented Mar 26, 2024

This adds a field to the trigger similar to the http-trigger that needs to be set if running preview1 components.

[[trigger.command]]
component = "sample"
# executor = { type = "preview1" }

This PR adds the ability for the trigger to support preview 2 components along with modules as well. It does it by attempting to load things as a component but falling back to attempting to load modules. The one other constraint it includes is that any preview 2 component supplied must target the wasi:cli world.

@radu-matei
Copy link
Member

Can we infer preview1 vs. preview2 without having to explicitly set it in the manifest?

@karthik2804
Copy link
Contributor Author

We can attempt to load as a component and fall back to a module. I just tested it locally, and it appears to work as expected.

@karthik2804 karthik2804 requested a review from radu-matei March 26, 2024 19:41
@karthik2804 karthik2804 marked this pull request as ready for review March 26, 2024 19:41
spin-app = { git = "https://github.com/fermyon/spin", tag = "v2.3.1" }
spin-core = { git = "https://github.com/fermyon/spin", tag = "v2.3.1" }
spin-trigger = { git = "https://github.com/fermyon/spin", tag = "v2.3.1" }
spin-app = { git = "https://github.com/fermyon/spin" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: leave a TODO to pin to the Spin 2.4 release for these dependencies.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to pin to 2.4?

I pinned to 2.3.1 earlier so this can be imported in the containerd shim.

Copy link
Contributor

@fibonacci1729 fibonacci1729 Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point and probably necessitates implementing this in terms of what's available in 2.3.1.

I only noticed the upgrade to the various new traits used below that will be made available in 2.4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can definitely be implemented pinned to 2.3.1 but I upgraded as I was following the example of the http-trigger. Do we think pinning it to 2.3.1 is a necessity or will the shim be updated to 2.4.0 once that is released?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll bump everything once 2.4 is out.

Signed-off-by: karthik2804 <[email protected]>
@radu-matei
Copy link
Member

Tested this with both Preview 1 and Preview 2 components and everything looks good. Merging so we can iterate. Thanks!

@radu-matei radu-matei merged commit 949972e into main Mar 29, 2024
@radu-matei radu-matei deleted the support_preview2 branch March 29, 2024 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants