Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Added Wazero dependent Wasm SDK #4821

Closed
wants to merge 3 commits into from
Closed

Added Wazero dependent Wasm SDK #4821

wants to merge 3 commits into from

Conversation

sky3n3t
Copy link

@sky3n3t sky3n3t commented Jun 28, 2022

Added a Wazero dependent Wasm SDK that is almost interchangeable with the Wasmtime dependent SDK.
Currently supports Go 1.17 and 1.18 as Wazero doesn't support Go 1.16

@srenatus srenatus self-requested a review June 28, 2022 07:59
@sky3n3t
Added Wazero dependent Wasm SDK
e8ed2f6 
Signed-off-by: Kaijlo <kaijserlove@gmail.com>
srenatus
srenatus reviewed Jun 28, 2022
View reviewed changes
Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

Had a first read through. This is cool stuff, thanks for working on it 👍

There are some large amounts of copied-over code here; which in itself is not a problem if refactoring is too cumbersome here -- but we should perhaps leave a few notes re: what was copied from where, and why.

Haven't had a chance to pull this down and play with it yet, but it the code looks mostly in a good shape. 🥳

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
features/wazero/wazero.go Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
internal/wasm/sdk/opa/opa_bench_test.go Outdated Show resolved Hide resolved
internal/wasm/wazero_sdk/internal/wazero/module.go Outdated Show resolved Hide resolved
internal/wasm/wazero_sdk/internal/wazero/module.go Outdated Show resolved Hide resolved
internal/wasm/wazero_sdk/internal/wazero/module.go Outdated Show resolved Hide resolved
internal/wasm/wazero_sdk/internal/wazero/VM.go Show resolved Hide resolved
internal/wasm/wazero_sdk/internal/wazero/builtins.go Outdated Show resolved Hide resolved
@sky3n3t sky3n3t force-pushed the main branch 6 times, most recently from 496b4c2 to 7f25c00 Compare June 29, 2022 10:02
srenatus
srenatus reviewed Jun 29, 2022
View reviewed changes
Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

Looks like some wires got crossed rebasing this? But I've noted many good improvements 👍 thanks for bearing with me.

docs/content/management-bundles.md Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
internal/wasm/wazero_sdk/internal/wazero/builtins.go Outdated Show resolved Hide resolved
plugins/rest/rest_auth.go Outdated Show resolved Hide resolved
@sky3n3t sky3n3t force-pushed the main branch 2 times, most recently from d150854 to 1092d6d Compare June 29, 2022 13:28
srenatus
srenatus reviewed Jun 30, 2022
View reviewed changes
Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

Still some git hurdles, it seems. Also go.sum and go.mod seem to have diverged, could you please rebase this again? 😃

docs/content/management-bundles.md Outdated Show resolved Hide resolved
test/cases/testdata/aggregates/test-aggregates-0001.yaml Outdated Show resolved Hide resolved
@sky3n3t
Copy link
Author

sky3n3t commented Jun 30, 2022

the differences in go.mod and go.sum seems to just be the wazero package

@sky3n3t sky3n3t force-pushed the main branch 2 times, most recently from 057cb1f to 000a071 Compare June 30, 2022 12:17
@sky3n3t
wazero SDK now supports unicode characters
cbbb855 
Added License
Signed-off-by: Kaijlo <kaijserlove@gmail.com>
srenatus
srenatus reviewed Jul 1, 2022
View reviewed changes
features/wazero/wazero.go Outdated
)

func init() {
opa.RegisterEngine("wazero", &factory{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use "wasm" here: it'll allow users of the golang package to pick which wasm engine they want: wazero or wasmtime, by importing features/wazero or features/wasm respectively.

@srenatus srenatus self-assigned this Jul 11, 2022
@srenatus
Copy link
Contributor

ℹ️ It might take a little longer, but I'm adopting this PR.

@srenatus
Copy link
Contributor

OK, I've got this. Closing the PR for now because our open PRs page is a mess 🧹

@srenatus srenatus closed this Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srenatus srenatus srenatus left review comments

Assignees

@srenatus srenatus

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sky3n3t @srenatus