feat(ext/kv): connect to remote database #20178
Conversation
| - SQLite - backed by a local SQLite database. This backend is suitable for | ||
| development and is the default when running locally. |
There was a problem hiding this comment.
FYI people have been asking about alternative local backends: #18975 can this be addressed somehow?
There was a problem hiding this comment.
Added a note on additional backends
There was a problem hiding this comment.
Can you add instructions on how to install protobuf for linux, windows, and osx as this will be a new requirement for those running "cargo install" or building. These should be in:
- cli/README,
- https://github.com/denoland/deno_install, and
- manual
losfair
force-pushed
the
kv-backend-remote
branch
from
b54c2b1 to
7793e7c
Compare
Added a test with a mock remote target in |
|
Added notes on protobuf compiler to related docs: |
test_util/src/lib.rs
Outdated
| @@ -72,6 +80,9 @@ const PORT: u16 = 4545; | |||
| const TEST_AUTH_TOKEN: &str = "abcdef123456789"; | |||
| const TEST_BASIC_AUTH_USERNAME: &str = "testuser123"; | |||
| const TEST_BASIC_AUTH_PASSWORD: &str = "testpassabc"; | |||
| const KV_DATABASE_ID: &str = "55bd5c48-58c2-4eb4-8adb-995c0763622a"; | |||
| const KV_ACCESS_TOKEN: &str = "578528e20364923229b08f94386b236f"; | |||
| const KV_DATABASE_TOKEN: &str = "iFOw5tcudKE2Cy4MKNuWZjxGkg4"; | |||
There was a problem hiding this comment.
I assume these are just mock values not real tokens? Can you make them more obviously such - for example:
const KV_DATABASE_ID: &str = "11111111-1111-1111-1111-111111111111";
const KV_ACCESS_TOKEN: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
const KV_DATABASE_TOKEN: &str = "MOCKMOCKMOCKMOCKMOCKMOCKMOCK";
There was a problem hiding this comment.
Can you add a test where the first endpoint returns the wrong JSON, and one where it returns the right structure but with a different protocol version?
| const db = await Deno.openKv( | ||
| "http://localhost:4545/kv_remote_authorize_invalid_format", | ||
| ); |
There was a problem hiding this comment.
I feel like we should try to get the metadata during Deno.openKv and throw already in the open call if we can't authenticate / the metadata response is wrong.
There was a problem hiding this comment.
That slows down app startup - any user initialization logic after Deno.openKv would have to wait for the first KV metadata response.
I prefer to keep that an asynchronous operation and defer any error to the first KV API call.
|
Just want to comment that this threw me for a loop when I pulled main and did a compile. I suspect a lot of folks used to building from source will scratch their head as the cargo build error don't give any hints as to how to correct it and I actually had to read through the PR to figure out what to do. Just has been years that On the other hand, this is an awesome cool feature! 👍 |
Maybe we should add a feature to switch the |
This patch adds a `remote` backend for `ext/kv`. This supports connection to Deno Deploy and potentially other services compatible with the KV Connect protocol.
This patch adds a
remotebackend forext/kv. This supports connection to Deno Deploy and potentially other services compatible with the KV Connect protocol.