Closed jakobo closed 3 days ago
Thanks for the PR! After quite a bit of thought, I'm going to be closing this in favour of #4.
I don't think "outgrowing" SQLite is a concern right now; some of the largest labelers with a 6-7 digit label count are currently running without issue off an SQLite database.
The other issue is that this would make it impossible to expose the default SQLite database for directly querying in a type-safe way without requiring projects to import and initialize libsql themselves. This is functionality that many existing labelers depend on, which I'm hesitant to change out from under them.
TL;DR: @libsql/client
accounts for remote databases, I don't foresee performance being an issue anytime soon, and this would significantly negatively impact existing usage patterns.
Motivation
This enables developers to "swap" in any database layer should they outgrow the default sqlite implementation. This also makes it possible to bring in sqlite alternatives like Turso which rely on network connections and therefore must be asynchronous.
The hooks/callback pattern remains a successful DB adapter pattern from Auth0, Clerk, and others, which do not dictate an ORM layer, leaving that to the person who is extending the core functionality.
Summary of Changes
DBCallbacks
type that describes the current set of database callbackssrc/util/sqlite.ts
for backwards compatibilityRisks
This exposes a new API which comes with backwards compatibility concerns. Adding new functionality to the labeler which also adds a new Database call becomes a breaking change. I don't think this is a concern, as people should be pinning their dependency on skyware.
Open Issues
getAllLabels
in the driver seems to be missing a limit and I can't for the life of me remember if that's automatically handled bystmt.iterate(cursor)
or not.DBCallbacks
to be part of the exports