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

libsql: Deasyncify some Conn trait methods #1135

Merged
merged 1 commit into from
Mar 5, 2024
Merged

libsql: Deasyncify some Conn trait methods #1135

merged 1 commit into from
Mar 5, 2024

Conversation

penberg
Copy link
Collaborator

@penberg penberg commented Mar 5, 2024

There's no reason to have is_autocommit(), changes() or last_insert_row() methods async because they're all fully local and cannot block. Deasyncify them.

@penberg penberg requested a review from haaawk March 5, 2024 11:53
Copy link
Contributor

@haaawk haaawk left a comment

Choose a reason for hiding this comment

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

LGTM but CI says you missed some places:

error[E0277]: `i64` is not a future
   --> libsql-server/tests/hrana/batch.rs:137:45
    |
137 |         assert_eq!(conn.last_insert_rowid().await, 1, "1st row id");
    |                                            -^^^^^
    |                                            ||
    |                                            |`i64` is not a future
    |                                            help: remove the `.await`
    |
    = help: the trait `futures_core::Future` is not implemented for `i64`
    = note: i64 must be a future or must implement `IntoFuture` to be awaited
    = note: required for `i64` to implement `std::future::IntoFuture`

error[E0[27](https://github.com/tursodatabase/libsql/actions/runs/8155781344/job/22292009232?pr=1135#step:6:28)7]: `i64` is not a future
   --> libsql-server/tests/hrana/batch.rs:143:45
    |
143 |         assert_eq!(conn.last_insert_rowid().await, 3, "3rd row id");
    |                                            -^^^^^
    |                                            ||
    |                                            |`i64` is not a future
    |                                            help: remove the `.await`
    |
    = help: the trait `futures_core::Future` is not implemented for `i64`
    = note: i64 must be a future or must implement `IntoFuture` to be awaited
    = note: required for `i64` to implement `std::future::IntoFuture`

error[E0277]: `i64` is not a future
   --> libsql-server/tests/hrana/batch.rs:147:45
    |
147 |         assert_eq!(conn.last_insert_rowid().await, 3, "last row id unchanged");
    |                                            -^^^^^
    |                                            ||
    |                                            |`i64` is not a future
    |                                            help: remove the `.await`
    |
    = help: the trait `futures_core::Future` is not implemented for `i64`
    = note: i64 must be a future or must implement `IntoFuture` to be awaited
    = note: required for `i64` to implement `std::future::IntoFuture`

error[E0277]: `bool` is not a future
   --> libsql-server/tests/standalone/mod.rs:[33](https://github.com/tursodatabase/libsql/actions/runs/8155781344/job/22292009232?pr=1135#step:6:34)9:38
    |
339 |         assert!(conn.is_autocommit().await);
    |                                     -^^^^^
    |                                     ||
    |                                     |`bool` is not a future
    |                                     help: remove the `.await`
    |
    = help: the trait `futures_core::Future` is not implemented for `bool`
    = note: bool must be a future or must implement `IntoFuture` to be awaited
    = note: required for `bool` to implement `std::future::IntoFuture`

error[E0277]: `bool` is not a future
   --> libsql-server/tests/standalone/mod.rs:[34](https://github.com/tursodatabase/libsql/actions/runs/8155781344/job/22292009232?pr=1135#step:6:35)3:39
    |
343 |         assert!(!conn.is_autocommit().await);
    |                                      -^^^^^
    |                                      ||
    |                                      |`bool` is not a future
    |                                      help: remove the `.await`
    |
    = help: the trait `futures_core::Future` is not implemented for `bool`
    = note: bool must be a future or must implement `IntoFuture` to be awaited
    = note: required for `bool` to implement `std::future::IntoFuture`

error[E0277]: `bool` is not a future
   --> libsql-server/tests/standalone/mod.rs:346:38
    |
346 |         assert!(conn.is_autocommit().await);
    |                                     -^^^^^
    |                                     ||
    |                                     |`bool` is not a future
    |                                     help: remove the `.await`
    |
    = help: the trait `futures_core::Future` is not implemented for `bool`
    = note: bool must be a future or must implement `IntoFuture` to be awaited
    = note: required for `bool` to implement `std::future::IntoFuture`

error[E0277]: `bool` is not a future
   --> libsql-server/tests/standalone/mod.rs:[35](https://github.com/tursodatabase/libsql/actions/runs/8155781344/job/22292009232?pr=1135#step:6:36)1:41
    |
351 |             assert!(!tx.is_autocommit().await);
    |                                        -^^^^^
    |                                        ||
    |                                        |`bool` is not a future
    |                                        help: remove the `.await`
    |
    = help: the trait `futures_core::Future` is not implemented for `bool`
    = note: bool must be a future or must implement `IntoFuture` to be awaited
    = note: required for `bool` to implement `std::future::IntoFuture`

error[E0277]: `bool` is not a future
   --> libsql-server/tests/standalone/mod.rs:352:42
    |
352 |             assert!(conn.is_autocommit().await); // connection is still autocommit
    |                                         -^^^^^
    |                                         ||
    |                                         |`bool` is not a future
    |                                         help: remove the `.await`
    |
    = help: the trait `futures_core::Future` is not implemented for `bool`
    = note: bool must be a future or must implement `IntoFuture` to be awaited
    = note: required for `bool` to implement `std::future::IntoFuture`

error[E0277]: `bool` is not a future
   --> libsql-server/tests/standalone/mod.rs:358:[38](https://github.com/tursodatabase/libsql/actions/runs/8155781344/job/22292009232?pr=1135#step:6:39)
    |
358 |         assert!(conn.is_autocommit().await);
    |                                     -^^^^^
    |                                     ||
    |                                     |`bool` is not a future
    |                                     help: remove the `.await`
    |
    = help: the trait `futures_core::Future` is not implemented for `bool`
    = note: bool must be a future or must implement `IntoFuture` to be awaited
    = note: required for `bool` to implement `std::future::IntoFuture`

For more information about this error, try `rustc --explain E02[77](https://github.com/tursodatabase/libsql/actions/runs/8155781344/job/22292009232?pr=1135#step:6:78)`.
error: could not compile `libsql-server` (test "tests") due to 9 previous errors
Error: The process '/home/runner/.cargo/bin/cargo' failed with exit code [101](https://github.com/tursodatabase/libsql/actions/runs/8155781344/job/22292009232?pr=1135#step:6:102)

There's no reason to have `is_autocommit()`, `changes()` or
`last_insert_row()` methods async because they're all fully local and
cannot block. Deasyncify them.
@penberg penberg added this pull request to the merge queue Mar 5, 2024
Merged via the queue into main with commit 6b13d77 Mar 5, 2024
16 checks passed
@penberg penberg deleted the deasyncify branch March 5, 2024 14:04
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