skyware-js / labeler

A lightweight alternative to Ozone for operating an atproto labeler.
Mozilla Public License 2.0
22 stars 5 forks source link

Switch to @libsql/client to support remote database connections #4

Open solimaniac opened 1 week ago

solimaniac commented 1 week ago

Problem

When deploying the labeler server on container platforms with ephemeral file systems (e.g., Heroku, Railway), the SQLite database file would get deleted on each deploy since the filesystem is not persistent. This makes it difficult to maintain label history across deployments

Solution

The @libsql/client supports both local SQLite files and remote database connections. This PR updates the labeler server to:

  1. Replace the previous SQLite client with @libsql/client
  2. Add optional database connection parameters:
    • url - Remote database URL (e.g., "libsql://your-db.skyware.js.org")
    • authToken - Authentication token for remote database

Usage


// Local SQLite (existing behavior)
const server = new LabelerServer({
  did: "did:example:123",
  signingKey: "your-key",
  dbPath: "labels.db"
});

// Remote database
const server = new LabelerServer({
  did: "did:example:123",
  signingKey: "your-key",
  url: "libsql://your-db.skyware.js.org",
  authToken: "your-auth-token"
});
solimaniac commented 1 week ago

Still need to test this with new & existing labelers to make sure it doesn't break anything

solimaniac commented 1 week ago

Managed to do some testing locally, was able to verify backwards compatibility with reading and writing to .db files created with the old database client, as well as fresh setups of the labeler

Issues found while testing remote connections:

@futurGH I'm currently wresting with the idea of introducing an async InitializeServer function to deal with the latter issue, this would however be a 'breaking' change to server setup for folks looking to upgrade from older versions

futurGH commented 1 week ago

Neat! Sorry I haven't had a chance to take a look at this yet, hoping to get to it this weekend.

  • The logic for creating a labels table in the database is now asynchronous, which means that upon successfully defining a new LabelerServer an operation immediately following that that requires the table may fail due to a race condition

I think the least breaking option here would be to introduce a dbLock promise as a class property that's awaited before anywhere the DB is accessed — not the cleanest solution but I don't like having "fake constructor" functions.

solimaniac commented 1 week ago

I think the least breaking option here would be to introduce a dbLock promise as a class property that's awaited before anywhere the DB is accessed — not the cleanest solution but I don't like having "fake constructor" functions.

Agreed, done!

Also added a warning when the server fails to set the journal_mode for the database, I suspect not all remote servers would expose it by default (some providers like Turso may be handling this out of the box)

futurGH commented 1 day ago

Finally getting a chance to take a look at this — is it ready for review?

solimaniac commented 1 day ago

Finally getting a chance to take a look at this — is it ready for review?

Yep, should be ready for review! Working on a separate PR to update the documentation as well