-
Notifications
You must be signed in to change notification settings - Fork 102
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
feat!: add ability to create plugins without an existing Context
#335
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…veral SDKs, ensure Context is threadsafe in Rust SDK
bhelx
approved these changes
May 17, 2023
bhelx
added a commit
that referenced
this pull request
May 19, 2023
## Breaking Changes * #315 HTTP calls will be disallowed by default now. If you want to enable HTTP you need to specify the hosts that the plug-in is allowed to communicate with. If you want to allow all hosts you can set it to `{allowed_hosts: ["*"]}` in the manifest. However, this isn't recommended unless you have some trust in the plug-in or are controlling the networking by some other means. * #335 In this PR we are creating an implicit context so people don't need to know about it if they don't care. In some languages function signatures have changed to make context an optional argument when creating a plug-in.
mhmd-azeez
pushed a commit
to extism/dotnet-sdk
that referenced
this pull request
Sep 14, 2023
## Breaking Changes * extism/extism#315 HTTP calls will be disallowed by default now. If you want to enable HTTP you need to specify the hosts that the plug-in is allowed to communicate with. If you want to allow all hosts you can set it to `{allowed_hosts: ["*"]}` in the manifest. However, this isn't recommended unless you have some trust in the plug-in or are controlling the networking by some other means. * extism/extism#335 In this PR we are creating an implicit context so people don't need to know about it if they don't care. In some languages function signatures have changed to make context an optional argument when creating a plug-in.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
EIP: extism/proposals#8
This PR makes minor breaking changes to several SDKs, but not to runtime C API. The threadsafety updates in the Rust SDK are kind of specific to Rust, I'm not sure if it makes sense to add the locks to all the other SDKs at this point. For the most part the
Context
andPlugin
types in the SDKs should be safe to use protected by a mutex but they aren't inherently threadsafe. That kind of locking should probably be done by the user.Send
andSync
implementations forContext
Plugin::call_map
to call a plugin and handle the output with the lock heldArc<Mutex<Plugin>>
between threadsPlugin::create
andPlugin::create_from_manifest
to create a plugin without aContext
Plugin
constructor to takecontext
as an optional named argument, to update usePlugin(data, context=context)
insteadPlugin
constructor to takecontext
as an optional named argument, to update usePlugin.new(data, context=context)
insteadNewPlugin
andNewPluginFromManifest
functionsPlugin
constructor to takecontext
as an optional named argument, to update usenew Plugin(data, wasi, config, host, context)
instead ofnew Plugin(context, data, wasi, functions, config)
(most people are probably usingcontext.plugin
instead of the Plugin constructor anyway)Plugin.create
andPlugin.of_manifest
to takecontext
as an optional named argument, to updatePlugin.create ~context data
andPlugin.of_manifest ~context data
insteadcreatePlugin
andcreatePluginFromManifest
functionsPlugin.new
to make a plugin without going throughContext.new_plugin
Plugin
constructors without aContext
argumentPlugin
constructor to take an optional context as the last argument, instead of requiring it to be the first argumentPlugin(wasm, wasi, functions, ctx)
instead ofPlugin(ctx, wasm, wasi, functions)
Plugin.create
andPlugin.createWithManifest
to create plugins in their own context.