Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Nice work!
Breaking changes by exposing async functions (instead of using block_on). Are we ok with that? asyncio is now widely used I believe.
I think exposing the async functionality is very good. I don't know if some users might prefer the synchronous functions and/or other async systems other than asyncio. I suppose the package/module version can be updated to reflect a breaking change.
[ ] Code duplication between the bindings crates (e.g. the WASM one). It is not problematic for now, and allows for more native interfaces but I thought I would mention it. But if it is we could merge most of the code in didkit directly and handle only the input/output in the bindings crates -- or maybe merge everything like we do for jni.
(merge crates (e.g. like we do with jni)? probably not, be inspired by Wasmer)
I agree adding a common set of functions in didkit would be good for reducing code duplication. At this point C and JNI are synchronous (using block_on), while WASM and this Python interface are async.
Wheel built successfully locally.
I'm running poetry install now which is taking a while. Edit: install completed, but test seems to be failing: https://github.com/spruceid/didkit-python/pull/1/files#r738597689
Sorry, something went wrong.
| Assuming the wheel is already built (see [../](../). | ||
|
|
||
| ```bash | ||
| $ poetry install | ||
| $ poetry run test | ||
| ``` |
There was a problem hiding this comment.
Install succeeds but test fails:
$ poetry install Installing dependencies from lock file Warning: The lock file is not up to date with the latest changes in pyproject.toml. You may be getting outdated dependencies. Run update to update them. Package operations: 10 installs, 0 updates, 0 removals • Installing pyparsing (2.4.7) • Installing attrs (21.2.0) • Installing iniconfig (1.1.1) • Installing packaging (21.0) • Installing pluggy (1.0.0) • Installing py (1.10.0) • Installing toml (0.10.2) • Installing pytest (6.2.5) • Installing didkit (0.3.0 /home/cel/src/didkit-python/target/wheels/didkit-0.3.0-cp39-cp39-linux_x86_64.whl) • Installing pytest-asyncio (0.15.1) Installing the current project: pydidkit_tests (0.1.0) $ poetry run test && echo ok $ echo $? 1
Sorry, something went wrong.
There was a problem hiding this comment.
Oops, can you try poetry run pytest? I've haven't been able to make it work since the renaming from pydidkit to didkit (poetry or pyenv installs the pypi version instead of the local wheel....)
Sorry, something went wrong.
There was a problem hiding this comment.
That works!
$ poetry run pytest =============================================================== test session starts =============================================================== platform linux -- Python 3.9.7, pytest-6.2.5, py-1.10.0, pluggy-1.0.0 rootdir: /home/cel/src/didkit-python/pydidkit_tests plugins: asyncio-0.15.1 collected 10 items src/pydidkit_tests/test_main.py .......... [100%] =============================================================== 10 passed in 0.15s ================================================================
Sorry, something went wrong.
|
CI tests failed for windows, but the error is unavailable. Is it working on Windows, or is that the only blocker for merging this? |
Sorry, something went wrong.
I'm just waiting for a new release of ssi/didkit. Windows compilation was fixed in spruceid/didkit#228 |
Sorry, something went wrong.
|
ssi v0.4.0 and didkit v0.4.0 are published: spruceid/didkit#269 spruceid/ssi#401 |
Sorry, something went wrong.
Original PR: spruceid/didkit#224
This PR rewrites the Python bindings, away from a wrapper around the raw C FFI, to a PyO3 project which allows to write the Python library in Rust (similar to WASM and wasm_bindgen) and handles the compilation/bundling (it's still converting to a C FFI behind the curtain). I also changed the tests to use pytest and made it an easy to use Python project.
Questions:
[ ] Breaking changes by exposing async functions (instead of using block_on). Are we ok with that? asyncio is now widely used I believe.
[ ] Code duplication between the bindings crates (e.g. the WASM one). It is not problematic for now, and allows for more native interfaces but I thought I would mention it. But if it is we could merge most of the code in didkit directly and handle only the input/output in the bindings crates -- or maybe merge everything like we do for jni.
(merge crates (e.g. like we do with jni)? probably not, be inspired by Wasmer)
[X] The python package/module is now called pydidkit because it is derived from the crate's name. Naming it didkit conflicts with the main library as they are in the same workspace. (didkit-wasm has the same issue.) In my opinion we should consider moving the bindings crates in their own repo (other similar projects do it this way, e.g. Wasmer).
- Kept the name didkit and moved to its own repository (this PR).
Todo list:
[X] Add CI/CD for all architectures.
Improvements for the future:
Example run for the CD pipeline https://github.com/spruceid/didkit-python/actions/runs/1338844709
Test release on PyPI https://pypi.org/project/didkit/0.0.2/