WASI imports should return a dict #581
Conversation
packages/api/src/wasi.rs
Outdated
| @@ -465,14 +465,14 @@ impl Environment { | |||
| /// import_object = wasi_env.generate_import_object(store, wasi.Version.SNAPSHOT1) | |||
| /// ``` | |||
| //#[pyo3(text_signature = "($self, store, wasi_version)")] | |||
| fn generate_import_object(&self, store: &Store, wasi_version: Version) -> ImportObject { | |||
| fn generate_import_object(&self, store: &Store, wasi_version: Version) -> PyResult<PyObject> { | |||
There was a problem hiding this comment.
Isn't this a breaking change? If we're OK making breaking changes then we should eliminate ImportObject from the python API entirely and just use dict everyone.
There was a problem hiding this comment.
Yeah, this is a slight small breaking change (on the return type), but all APIs behave the same (with the exception of not being able to call register on the WASI Import Object, although I think that use case is very limited and I doubt people were using it)
I was thinking on removing the ImportObject, but if we do that we will make invalid a lot of existing code that rely on Wasmer Python. I think we can just mark it as deprecated for now.
There was a problem hiding this comment.
I would prefer if we had a separate function which returns a dict, while keeping the existing one. This would avoid any breaking changes.
We can always clean this up later when we make a new release with breaking changes (3.0?).
There was a problem hiding this comment.
Sounds reasonable. Can you handle this from here?
packages/api/src/import_object.rs
Outdated
| @@ -186,6 +200,43 @@ impl ImportObject { | |||
| self.inner.contains_namespace(namespace_name) | |||
| } | |||
|
|
|||
| /// Gets an `ImportObject` from a Python dictionary. | |||
There was a problem hiding this comment.
This comment seems to be the wrong way around.
Amanieu
force-pushed
the
wasi-imports
branch
from
1a13797 to
6c988f0
Compare
This PR depends on this:
wasmerio/wasmer#2726