Skip to content

feat: Add libSQL adapter (proposal)#240

Open
KnightNiwrem wants to merge 3 commits intomainfrom
add-libsql
Open

feat: Add libSQL adapter (proposal)#240
KnightNiwrem wants to merge 3 commits intomainfrom
add-libsql

Conversation

@KnightNiwrem
Copy link
Copy Markdown

Please help verify (not familiar with lerna and pnpm) this works for deno and node.

@KnightNiwrem KnightNiwrem changed the title Proposal: Add libSQL adapter feat: Add libSQL adapter (proposal) Feb 7, 2025
Copy link
Copy Markdown
Member

@KnorpelSenf KnorpelSenf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting stuff. @Satont?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 8? It should be 9

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "why" is because pnpm lock version was 6 at storages@2.4.2. Now that lockfile is version 9, will update

Ref: https://github.com/pnpm/spec/tree/fd3238639af86c09b7032cc942bab3438b497036/lockfile#relation-of-pnpm-version-to-lockfile-version

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use version 9.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why example not using sessions?

Those packages suposed to be used mostly in sessions. Yea, not only sessions, but certainly not for such use in bare code.

Also for deno i'd like to see env usage for bot token, tursor url and token.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for nodejs as for deno.

Comment on lines +23 to +35
static async create<T>(opts: { client: Client, table: string }) {
const createTableStatement = `
CREATE TABLE IF NOT EXISTS "${opts.table}" (
key TEXT NOT NULL,
value TEXT
);`;
await opts.client.execute(createTableStatement);

const createIndexStatement = `CREATE UNIQUE INDEX IF NOT EXISTS "IDX_${opts.table}" ON "${opts.table}" (key);`;
await opts.client.execute(createIndexStatement);

return new LibSQLAdapter<T>(opts);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some users have correctly pointed out that await create() to be called on every application start is bad, because sometimes bots can be used in serverless environments where this call takes a decent amount of time. Instead, I'd like to instruct them in the readme that they should create a table in their database. Then we will accept the table name, client, as an option. And we wouldn't need a static create method

Thats also about psql adapter, which i didnt refactored yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants