feat(resources): add support for turso client w/o provisioning #996

Kazy · 2023-06-12T16:20:10Z

Description of change

This adds support for Turso but without provisioning.

Close #986.

How has this been tested? (if applicable)

Locally yes, both with a local and a remote database.

TODO

Clarify the few XXX in the code.

resources/turso/src/lib.rs

iulianbarbu

Thanks @Kazy ! This looks on the right path. I would need to take a closer look at the tests and do a manual one also. Consider this part 1. :D

We'll be a bit low on bandwidth reviewing this during the next two weeks because we're planning for a bigger release and an off-site soon after. We'll try our best to provide meaningful feedback though in the meantime, so please shoot if you have other qs.

iulianbarbu · 2023-06-16T12:55:10Z

resources/turso/README.md

+
+```rust
+#[shuttle_runtime::main]
+async fn app(#[shuttle_turso::Turso(addr="advanced-lightspeed.turso.io", auth_token=Some("token")] client: Client) -> __ { }


Is the token provided in plain?

It was, and I've updated the code to read from the secrets instead.

resources/turso/src/lib.rs

oddgrd

Thank you @Kazy, this is really great! Apologies for the delayed review, but I'd say this is very nearly finished, I just have some suggestions for some small changes we could make. Thanks for adding tests and docs too ❤️

oddgrd · 2023-06-23T11:24:17Z

resources/turso/Cargo.toml

+version = "0.18.0"
+edition = "2021"
+license = "Apache-2.0"
+description = "Plugin to obtain a client connected to a Turso database"
+keywords = ["shuttle-service", "turso"]
+
+[dependencies]
+async-trait = "0.1.56"
+libsql-client = { version = "0.27" }
+serde = { version = "1.0.148", features = ["derive"] }
+shuttle-service = { path = "../../service", version = "0.18.0", default-features = false }


Just some delayed-review version bumps 😄

Suggested change

version = "0.18.0"

edition = "2021"

license = "Apache-2.0"

description = "Plugin to obtain a client connected to a Turso database"

keywords = ["shuttle-service", "turso"]

[dependencies]

async-trait = "0.1.56"

libsql-client = { version = "0.27" }

serde = { version = "1.0.148", features = ["derive"] }

shuttle-service = { path = "../../service", version = "0.18.0", default-features = false }

version = "0.19.0"

edition = "2021"

license = "Apache-2.0"

description = "Plugin to obtain a client connected to a Turso database"

keywords = ["shuttle-service", "turso"]

[dependencies]

async-trait = "0.1.56"

libsql-client = { version = "0.27" }

serde = { version = "1.0.148", features = ["derive"] }

shuttle-service = { path = "../../service", version = "0.19.0", default-features = false }

oddgrd · 2023-06-23T11:28:50Z

resources/turso/README.md

+use libsql_client::client::Client;
+
+#[shuttle_runtime::main]
+async fn app(#[shuttle_turso::Turso(addr="libsql://advanced-lightspeed.turso.io")] client: Client) -> __ { }


We can use secrets string interpolation here, see this section in the rds docs for an example. I think we should also add the token param to the example and interpolate it from secrets, and maybe also make that one required.

I'm also thinking we should use one of the frameworks for the example so it's clear that using turso resource doesn't change the return type, what do you think?

Suggested change

async fn app(#[shuttle_turso::Turso(addr="libsql://advanced-lightspeed.turso.io")] client: Client) -> __ { }

async fn app(#[shuttle_turso::Turso(addr="libsql://advanced-lightspeed.turso.io")] client: Client) -> ShuttleAxum { ... }

Perhaps we don't need to use secrets interpolation for the address, since unlike an sql connection string it doesn't contain a password. But for the token I think it would be good

Ah I wasn't aware that we could interpolate it like that ! This is better than my solution that requires the user to use a Secret.toml file; one can just use a hardcoded token when playing with it.

Yes, Pieter implemented it a while ago but we haven't really documented it very clearly. 😄 It's pretty cool!

oddgrd · 2023-06-23T11:31:41Z

resources/turso/src/lib.rs

+impl Turso {
+    async fn output_from_addr(
+        &self,
+        factory: &mut dyn Factory,
+        addr: &str,
+    ) -> Result<<Turso as ResourceBuilder<Client>>::Output, shuttle_service::Error> {
+        match factory
+            .get_secrets()
+            .await
+            .expect("secrets should be available")
+            .get(&self.token_secret)
+        {
+            Some(token) => Ok(TursoOutput {
+                conn_url: Url::parse(addr).map_err(Error::UrlParseError)?,
+                token: Some(token.to_string()),
+            }),
+            None => Err(ShuttleError::Custom(CustomError::msg(format!(
+                "could't find secret {}",
+                self.token_secret
+            )))),
+        }
+    }
+}


I'm thinking maybe we can replace this with the secrets string interpolation?

oddgrd · 2023-06-23T11:37:59Z

resources/turso/README.md

+**IMPORTANT**: Currently Shuttle isn't able to provision a database for you (yet). This means you will have to create an account on their [website](https://turso.tech/) and follow the few steps required to create a database and create a token to access it.
+
+Add `shuttle-turso` to the dependencies for your service.
+This resource will be provided by adding the `shuttle_turso::Turso` attribute to your Shuttle `main` decorated function.


Excellent docs, thank you! Perhaps we can suggest to cargo add shuttle-turso?

…n local

…README

Kazy · 2023-06-23T13:54:03Z

Sorry for the forced push. I missed that you updated the fork and ended up with some unwanted commits, and I preferred to have a clean state :)

oddgrd

This looks great, thank you @Kazy! I left a couple of comments but they are just nits 😄

oddgrd · 2023-06-23T16:33:15Z

resources/turso/src/lib.rs

+        Self {
+            addr: "".to_string(),
+            token: "".to_string(),
+            local_addr: None,
+        }


If we derive Default for the Turso struct, we can do this:

Suggested change

Self {

addr: "".to_string(),

token: "".to_string(),

local_addr: None,

}

Self::default()

oddgrd · 2023-06-23T17:22:01Z

resources/turso/README.md

+use libsql_client::client::Client;
+


Suggested change

use libsql_client::client::Client;

use libsql_client::client::Client;

use shuttle_axum::ShuttleAxum;

…ADME

resources/turso/Cargo.toml

…le-hq#996) * feat(resources): add support for turso client w/o provisioning * chore(resources/turso): remove todo about checking for local db file * test(resources/turso): add test * build: add missing shuttle-turso override of cargo path * ci: add shuttle-turso to the test matrix * ref: unnecessary borrow in ResourceBuilder config * ref(resources/turso): read secrets from Secrets.toml, support token in local * fix(resources/turso): please clippy * build(resources/turso): bump versions * ref(resources/turso): rely on secret interpolation for token, update README * ref(resources/turso): use default for Turso, add missing import in README * chore: upgrade and lock libsql version * chore: v0.20.0 * chore: v0.20.0 --------- Co-authored-by: oddgrd <[email protected]>

Kazy force-pushed the turso-integration branch from 0132f6b to ce916a5 Compare June 12, 2023 16:22

AlphaKeks reviewed Jun 13, 2023

View reviewed changes

resources/turso/src/lib.rs Outdated Show resolved Hide resolved

AlphaKeks reviewed Jun 13, 2023

View reviewed changes

resources/turso/src/lib.rs Outdated Show resolved Hide resolved

Kazy marked this pull request as ready for review June 13, 2023 14:57

iulianbarbu reviewed Jun 16, 2023

View reviewed changes

Kazy requested a review from iulianbarbu June 20, 2023 16:07

oddgrd reviewed Jun 23, 2023

View reviewed changes

Kazy added 10 commits June 23, 2023 15:51

feat(resources): add support for turso client w/o provisioning

e9d9e5a

chore(resources/turso): remove todo about checking for local db file

30e5853

test(resources/turso): add test

cc16b17

build: add missing shuttle-turso override of cargo path

0782eb9

ci: add shuttle-turso to the test matrix

1b5b760

ref: unnecessary borrow in ResourceBuilder config

33c651e

ref(resources/turso): read secrets from Secrets.toml, support token i…

cc6e3e5

…n local

fix(resources/turso): please clippy

f175f9e

build(resources/turso): bump versions

e9402b1

ref(resources/turso): rely on secret interpolation for token, update …

df0da6c

…README

Kazy force-pushed the turso-integration branch from 7b57599 to df0da6c Compare June 23, 2023 13:51

Kazy requested a review from oddgrd June 23, 2023 13:55

oddgrd approved these changes Jun 23, 2023

View reviewed changes

ref(resources/turso): use default for Turso, add missing import in RE…

4e56e39

…ADME

Kazy requested a review from oddgrd June 23, 2023 20:49

Kazy mentioned this pull request Jun 26, 2023

feat: add turso example shuttle-hq/shuttle-examples#67

Merged

chore: upgrade and lock libsql version

49a5c83

oddgrd reviewed Jun 28, 2023

View reviewed changes

resources/turso/Cargo.toml Outdated Show resolved Hide resolved

chore: v0.20.0

ebb4058

oddgrd reviewed Jun 28, 2023

View reviewed changes

resources/turso/Cargo.toml Outdated Show resolved Hide resolved

chore: v0.20.0

10f7e7a

oddgrd merged commit 4ea9883 into shuttle-hq:main Jun 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(resources): add support for turso client w/o provisioning #996

feat(resources): add support for turso client w/o provisioning #996

Kazy commented Jun 12, 2023 •

edited

Loading

iulianbarbu left a comment •

edited

Loading

iulianbarbu Jun 16, 2023

Kazy Jun 17, 2023

oddgrd left a comment

oddgrd Jun 23, 2023

oddgrd Jun 23, 2023

oddgrd Jun 23, 2023 •

edited

Loading

Kazy Jun 23, 2023

oddgrd Jun 23, 2023

oddgrd Jun 23, 2023

oddgrd Jun 23, 2023

Kazy commented Jun 23, 2023

oddgrd left a comment

oddgrd Jun 23, 2023

oddgrd Jun 23, 2023

	async fn app(#[shuttle_turso::Turso(addr="libsql://advanced-lightspeed.turso.io")] client: Client) -> __ { }
	async fn app(#[shuttle_turso::Turso(addr="libsql://advanced-lightspeed.turso.io")] client: Client) -> ShuttleAxum { ... }

feat(resources): add support for turso client w/o provisioning #996

feat(resources): add support for turso client w/o provisioning #996

Conversation

Kazy commented Jun 12, 2023 • edited Loading

Description of change

How has this been tested? (if applicable)

TODO

iulianbarbu left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oddgrd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oddgrd Jun 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kazy commented Jun 23, 2023

oddgrd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kazy commented Jun 12, 2023 •

edited

Loading

iulianbarbu left a comment •

edited

Loading

oddgrd Jun 23, 2023 •

edited

Loading