libsql: Switch to hyper for local offline #1825
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LucioFranco
changed the title
haaawk
reviewed
Nov 18, 2024
This commit switches our usage of reqwest to hyper. In the future, this will allow us to pass in configuration for the connector to support both custom TLS implementations (aka using the system tls impl over rustls) and connecting to the sqld network simulation for future testing. This also removes the extra dependency on reqwest which is not needed since we already pull in the dependencies needed to implement the new protocol for local offline writes.
LucioFranco
force-pushed
the
lucio/sync-switch-hyper
branch
from
b190f80 to
c8f74e4
Compare
haaawk
approved these changes
Nov 18, 2024
| // TODO(lucio): convert this to use bytes to make this clone cheap, it should be | ||
| // to possible use BytesMut when reading frames from the WAL and efficiently use Bytes | ||
| // from that. | ||
| let req = req.body(frame.clone().into()).expect("valid body"); |
There was a problem hiding this comment.
body wouldn't accept frame.to_vec()?
There was a problem hiding this comment.
I found to_vec to not be clear since, its already a Vec<u8> so this instead reads copy the frame and convert it into the type accepted by body (which in this case is hyper::Body). This should still compile to the same thing.
penberg
added a commit
that referenced
this pull request
Nov 19, 2024
…-hyper" This reverts commit 265bd83, reversing changes made to 909b8af because it breaks the example app: ``` thread 'main' panicked at /Users/penberg/src/tursodatabase/libsql/libsql/src/local/database.rs:502:49: called `Result::unwrap()` on an `Err` value: hyper::Error(Connect, "invalid URL, scheme is not http") note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit switches our usage of reqwest to hyper. In the future, this will allow us to pass in configuration for the connector to support both custom TLS implementations (aka using the system tls impl over rustls) and connecting to the sqld network simulation for future testing. This also removes the extra dependency on reqwest which is not needed since we already pull in the dependencies needed to implement the new protocol for local offline writes.