-
Notifications
You must be signed in to change notification settings - Fork 14
feat(mdk,gate): hostcall #706
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
Conversation
zifeo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, the simple json protocol is more than enough. Shall we also test that in python and typescript?
|
@CodiumAI-Agent /review |
PR Review 🔍
Code feedback:
|
3ea6386 to
8fe254a
Compare
WalkthroughThe updates encompass a broad range of changes across various files and modules, focusing on version upgrades, functional enhancements, and the introduction of new features. Key adjustments include the implementation of new interfaces and methods related to WASI standards, enhancements in error handling and GraphQL functionalities, and the integration of authentication mechanisms. These changes aim to improve system compatibility, extend functionality, and enhance user and developer interactions with the applications. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Out of diff range and nitpick comments (13)
typegate/engine/runtime.d.ts (1)
58-58: Ensure the callback function is properly documented.Consider adding TypeScript documentation comments above the
cbparameter to explain its purpose, expected behavior, and any side effects it might have. This will improve code readability and maintainability.typegate/src/runtimes/wit_wire/mod.ts (1)
9-12: Consider removing the debug function in production code.The
dbgfunction is used for debugging purposes. If this code is meant for production, consider removing such functions or ensuring they are disabled or appropriately secured to avoid verbose logging in production environments.wit/deps/io/streams.wit (1)
64-69: Ensure consistency in blocking behavior descriptions.The description of
blocking-readcould be enhanced by explicitly stating how it differs fromreadin terms of blocking behavior, even though it mentions "Except for blocking, behavior is identical toread." This could improve clarity for users of the API.wit/deps/sockets/udp.wit (1)
155-168: Check the behavior of buffer size settings.The functions
set-receive-buffer-sizeandset-send-buffer-sizemention that values might be clamped or rounded, but do not specify how this behavior can be observed or verified by the caller. Adding a method to retrieve the actual set value could enhance usability and transparency.typegate/engine/src/runtimes/wit_wire.rs (1)
Line range hint
58-64: Consider avoiding unsafe code unless absolutely necessary.- unsafe { - let mut cwasm_bytes = vec![]; - zstd::stream::copy_decode(&cwasm_zst_bytes[..], &mut cwasm_bytes) - .map_err(|err| format!("error decompressing serialized component: {err}"))?; - Component::deserialize(&engine, cwasm_bytes) - .map_err(|err| format!("error loading pyrt serialized component: {err}")) - } + // Consider refactoring to avoid unsafe code or ensure that all safety invariants are met.wit/deps/filesystem/types.wit (8)
35-35: Consider updating the documentation to clarify the renaming fromfiletypetodescriptor-type.
58-58: Update the documentation to clarify the renaming fromfdflagstodescriptor-flags.
101-101: Update the documentation to clarify the renaming fromfilestattodescriptor-stat.
360-360: Clarify the renaming fromfd_filestat_set_sizetoset-sizein the documentation.
367-367: Clarify the renaming fromfd_filestat_set_timestoset-timesin the documentation.
444-444: Clarify the renaming fromfd_filestat_gettostatin the documentation.
453-453: Clarify the renaming frompath_filestat_gettostat-atin the documentation.
466-466: Clarify the renaming frompath_filestat_set_timestoset-times-atin the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Out of diff range and nitpick comments (1)
typegate/engine/src/runtimes/wit_wire.rs (1)
Line range hint
65-71: Consider replacing unsafe code with safer alternatives.- unsafe { - let mut cwasm_bytes = vec![]; - zstd::stream::copy_decode(&cwasm_zst_bytes[..], &mut cwasm_bytes) - .map_err(|err| format!("error decompressing serialized component: {err}"))?; - Component::deserialize(&engine, cwasm_bytes) - .map_err(|err| format!("error loading pyrt serialized component: {err}")) - } + // Suggested to encapsulate unsafe blocks more securely or find a safe alternative.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #706 +/- ##
==========================================
- Coverage 73.60% 73.43% -0.18%
==========================================
Files 121 121
Lines 14187 14295 +108
Branches 1416 1420 +4
==========================================
+ Hits 10443 10497 +54
- Misses 3717 3771 +54
Partials 27 27 ☔ View full report in Codecov by Sentry. |
michael-0acf4
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm so far
zifeo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You just earned a new title: the boundary wizard, very nice! 🪄
fix: cherry-pick feat(wasm): wit component support test(wasm): error propagation refactor(wasm): all remaining wasmedge -> wasm fix(wasm): bad conversion feat(wasm): nested object output test(wasm): tuple deserialize test(wasm): update binding test value feat(wasm): nested wit input support feat(wasm): handle enum input, fix object bug feat(wasm): reject on extra fields fix(wasm): typos and minor cleanups fix(tests.yml): disable cache deno dir for now fix: version lock fix: lockfile refactor: rename all wasmedge ref. to wasm refactor(gate): wasi 0.2 pyrt refactor: rename to `pyrt_wit_wire` fix: pre-commit issue wip: try ci fix feat: add pyrt bin inline refactor: `python_wasi` -> `python` wip: import module support fix: rebase bugs feat(metagen): `mdk_rs` finalization (#673) - Finalizes the host side of the `wit_wire` implementation started in - Fixes a few issues with the `mdk_rust` generator. The code generator was already in place but we the typegate had no support for the `wit_wire` interface used by the rust mdk. This PR adds that. The two week delay is mainly due to the base work required in #669 and related PRs. _No breaking changes on user._ - [x] The change come with new or modified tests - [x] Hard-to-understand functions have explanatory comments - [ ] End-user documentation is updated to reflect the change feat: Enable batch prisma queries in the typegate runtime (#682) Enable batch prisma queries (and transaction) in the typegate runtime Console [MET-381](https://linear.app/metatypedev/issue/MET-381/console-collections) <!-- Explain HOW users should update their code when required --> - [x] The change come with new or modified tests - [ ] Hard-to-understand functions have explanatory comments - [ ] End-user documentation is updated to reflect the change --------- Co-authored-by: Teo Stocco <teo@zifeo.com> Co-authored-by: Yohe-Am <56622350+Yohe-Am@users.noreply.github.com> fix: update poetry lockfile fix: CI issues fix: minor bugs fix: ci breakage wip: try fix
4a40708 to
afad9a2
Compare
Introduces a mechanism for wasm materializers to access hostgate functions.
This implements a pretty basic JSON wire interface, a singular
hostcallfunction that's exposed to materializers. The only implemented function on this interface aregqlqueries.This is a stacked PR on top of #687.
MET-473.
Summary by CodeRabbit
New Features
std_urland a new task for installing WASI adapter related files.Enhancements
Bug Fixes
Documentation
Refactor