Skip to content
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

Add support for LibSQL remote #11385

Merged
merged 13 commits into from
Aug 28, 2024
Merged

Conversation

Fryuni
Copy link
Member

@Fryuni Fryuni commented Jul 1, 2024

Changes

  • Allows passing a URL to ASTRO_DB_REMOTE_URL pointing to a LibSQL server instance (using any supported LibSQL protocols), to a file, or to an in-memory DB. This enables Astro DB to be used with self-hosting and air-gapped deployments.
  • Accepts extra LibSQL options as query parameters in the remote URL.
    • For example, memory:?syncUrl=libsql%3A%2F%2Fdb-server.example.com would create an in-memory embedded replica for the LibSQL DB on libsql://db-server.example.com.

Testing

Replicated the test for Studio remote using the LibSQL remote

Docs

Copy link

changeset-bot bot commented Jul 1, 2024

🦋 Changeset detected

Latest commit: 9d0d588

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

@Adammatthiesen Adammatthiesen left a comment

Choose a reason for hiding this comment

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

Amazing how simple the changes that are required to unlock the potential of this library, I could see many people very happy about this change! since it will also allow more "Self-hosted-production" database options!

@syhily
Copy link
Contributor

syhily commented Jul 1, 2024

I'm very happy and confident that this PR unlocks new use cases for Astro DB. I hope it gets merged soon.

@ematipico
Copy link
Member

Thank you @Fryuni. I'll make sure that Core is aware of this PR :)

@matthewp
Copy link
Contributor

matthewp commented Jul 1, 2024

How is this working? Is this because the libsql db has an https URL? Any links you can share? Seems exciting.

@Fryuni
Copy link
Member Author

Fryuni commented Jul 1, 2024

@matthewp

How is this working?

It uses the DB itself to manage the snapshots, like Studio did back in the closed beta.

Is this because the libsql db has an https URL?

LibSQL has two APIs, one exclusively over HTTP and one using the Hrana protocol, which can work both over HTTP or over WebSockets or a combination of both.

The Hrana protocol allows for stateful connections, so transactions are supported.

Any links you can share? Seems exciting.

Besides the specs above, the supported URL protocols are described in LibSQL's type docs. It links to a section of the README that was removed during some reorganization of the repo a few months ago.

The LibSQL client uses the Hrana protocol. The given URL can have http, https, ws or wss as the protocol to select which underlying transport should be used or can be libsql and let the library decide which transport to use (currently selects HTTP). It can also be file to use a local DB as we do for dev or the exact string :memory: to use a DB without a backing file for persistence (nice for testing and as replica for a remote DB).

Since Studio already uses http/https I added the variation of libsql+http and libsql+https to select a LibSQL remote using an explicit HTTP transport.

@Fryuni
Copy link
Member Author

Fryuni commented Jul 1, 2024

To test it out, I have this route deployed using this branch and configured to use an embedded in-memory replica synchronizing with Turso.

Working perfectly :)

@matthewp
Copy link
Contributor

matthewp commented Jul 2, 2024

@Fryuni Fred is out this week, he has the most context for how migrations work, so when he gets back he'll review this and leave any feedback.

@matthewp
Copy link
Contributor

@Fryuni Plan is still to have this in for 4.13

@matthewp matthewp added this to the 4.13 milestone Jul 22, 2024
@sarah11918
Copy link
Member

Just noting that if this goes through, we will want docs for how someone can do this in the DB guide!

@matthewp
Copy link
Contributor

!preview db-remote

@sarah11918
Copy link
Member

Hey @Fryuni ! Friendly ping to remind you that I'd normally want to have a docs PR reviewed before approving an astro PR, so I know how to make helpful suggestions to the changeset here! And, this content is what helps Erika write the release blog post so she knows how to hype your feature!

Since PRs will be merged tomorrow for the minor, can I help you in any way to get a docs PR together today?

@Fryuni
Copy link
Member Author

Fryuni commented Jul 30, 2024

I got caught up with some things recently, but I'll have a PR for docs today 😁

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Approving for docs! 🥳

Comment on lines 453 to 455
if (error.code === 'SQLITE_UNKNOWN') {
// Snapshots table was not created yet,
return;
Copy link
Member

Choose a reason for hiding this comment

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

Can you expand the current comment and explain why this error doesn't need to be handled? Still, I think it's valuable to have a log of the error.

packages/db/src/core/cli/migration-queries.ts Outdated Show resolved Hide resolved
packages/db/src/core/utils.ts Outdated Show resolved Hide resolved
packages/db/src/runtime/db-client.ts Outdated Show resolved Hide resolved
packages/db/src/runtime/db-client.ts Show resolved Hide resolved
packages/db/src/runtime/db-client.ts Outdated Show resolved Hide resolved
matthewp
matthewp previously approved these changes Jul 31, 2024
@matthewp
Copy link
Contributor

It does seem a bit odd to me to reuse the environment variables for Studio to connect to something non-studio. Can we not have different variables for this use-case? Would that be a problem?

@matthewp matthewp dismissed their stale review July 31, 2024 14:07

Think a little more work needed here

@Princesseuh
Copy link
Member

We'll be releasing this outside of the minor, as there's still some concerns to be addressed.

@Princesseuh Princesseuh removed this from the 4.13 milestone Aug 1, 2024
[skip ci]

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
@Fryuni
Copy link
Member Author

Fryuni commented Aug 1, 2024

It does seem a bit odd to me to reuse the environment variables for Studio to connect to something non-studio. Can we not have different variables for this use-case? Would that be a problem?

I explained that on Discord to Sarah, but it might have gotten lost there:

Since the Studio side is closed source, I don't know what would break internally if I changed the name of the env var. If it were just DB, I'd have renamed it to something like ASTRO_DB_REMOTE_URL and ASTRO_DB_TOKEN.
So I kept the same name to not break anything in Studio.


I'll refactor the code to accept different values, here is what I'm thinking:

  • Add a new ASTRO_DB_REMOTE_URL, which would be used just for LibSQL support.
    • If only ASTRO_STUDIO_REMOTE_URL is present, use that URL to connect as a Studio backend.
    • If only ASTRO_DB_REMOTE_URL is present, use that URL to connect to LibSQL. The scheme would no longer need to be modified.
    • If neither env is present, use the prod Studio URL.
    • If both env vars are present, use the Studio one.
  • When connecting to Studio, the token is read from ASTRO_STUDIO_TOKEN
  • When connecting to LibSQL, the token is read from ASTRO_DB_TOKEN

This way, it should not change anything about configuring the Studio connection while also adding LibSQL support.

What do you guys think?

@ematipico
Copy link
Member

@Fryuni I like the proposal! :)

@matthewp
Copy link
Contributor

@Fryuni sorry for the late reply, yes that sounds good! Basically just treat this as a new feature that's unrelated to the Astro Studio flow. So your proposal sounds good.

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Still approving for docs, and wondering whether this will be included in 4.15!

@ematipico ematipico added this to the 4.15 milestone Aug 26, 2024
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@ematipico ematipico merged commit d6611e8 into withastro:main Aug 28, 2024
11 of 13 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Aug 29, 2024
@Fryuni Fryuni deleted the feat/db-libsql-remote branch August 31, 2024 17:33
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.

8 participants