-
Notifications
You must be signed in to change notification settings - Fork 322
Add basic support for sqlite-backed durable objects #723
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
base: main
Are you sure you want to change the base?
Add basic support for sqlite-backed durable objects #723
Conversation
This looks fantastic!! |
Fixes cloudflare#645 Co-authored-by: Antonio Scandurra <[email protected]>
1a9b519
to
e0a401e
Compare
I say wrangler 4.10.0 (update available 4.11.0) and thought WOW it's been approved - awesome.....but then came to see this is still sitting unreviewed. 👎 |
You might want to try to get in touch with one of the developers that get their PRs accepted to see if you can get merged. |
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.
Another nice addition to this PR would be some tests/example code in worker-sandbox, e.g., c4107bf. And an update to the README at https://github.com/cloudflare/workers-rs?tab=readme-ov-file#durable-objects to use new_sqlite_classes
, etc.
(I'm not a maintainer on this repo, but that should help to get this merged quickly.)
pub enum SqlStorageValue { | ||
Null, | ||
Boolean(bool), | ||
Integer(i32), |
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.
SQLite integers can be up to 8 bytes (https://sqlite.org/datatype3.html). Any reason not to use i64
here?
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.
We can't losslessly convert to number
from an i64
and the DO sql api doesn't seem to support using BigInt
for values above Number.MAX_SAFE_INTEGER
. I'll ask the DO team for clarification to see if we can support this
row.and_then(|row| serde_wasm_bindgen::from_value(row).map_err(JsValue::from)) | ||
.map_err(Error::from), | ||
) | ||
} |
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.
Any reason not to add support for toArray
and raw
as well? https://developers.cloudflare.com/durable-objects/api/storage-api/#returns
#[wasm_bindgen] | ||
extern "C" { | ||
#[wasm_bindgen(extends=js_sys::Object)] | ||
pub type DurableObjectSqlStorage; |
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.
We should name this SqlStorage
use wasm_bindgen::prelude::*; | ||
|
||
#[wasm_bindgen] | ||
extern "C" { |
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.
There are few more methods for this class that we should implement: https://workers-types.pages.dev/#SqlStorage
} | ||
|
||
#[wasm_bindgen] | ||
extern "C" { |
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.
Few more methods that we should implement: https://workers-types.pages.dev/#SqlStorageCursor
pub enum SqlStorageValue { | ||
Null, | ||
Boolean(bool), | ||
Integer(i32), |
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.
We can't losslessly convert to number
from an i64
and the DO sql api doesn't seem to support using BigInt
for values above Number.MAX_SAFE_INTEGER
. I'll ask the DO team for clarification to see if we can support this
Fixes #645
Hi, we want to use sqlite-backed durable objects using Rust, so we added some bindings to the Sql Storage API. Let us know if there are changes you'd like to see. Thanks.