Update remote_encryption builder to take encryption object instead of option#2127
Merged
avinassh merged 2 commits intotursodatabase:mainfrom Jul 13, 2025
Merged
Update remote_encryption builder to take encryption object instead of option#2127avinassh merged 2 commits intotursodatabase:mainfrom
remote_encryption builder to take encryption object instead of option#2127avinassh merged 2 commits intotursodatabase:mainfrom
Conversation
remote_encryption builder to take encryption object instead of option
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the builder API so that remote_encryption takes an EncryptionContext directly instead of an Option<EncryptionContext>, and updates all call sites and examples to match.
- Change
remote_encryptionsignature in all builder impls to acceptEncryptionContextand wrap it inSome(...). - Update the sync protocol code to conditionally call
remote_encryptionwhen anOption<EncryptionContext>is present. - Revise the example (
encryption_sync.rs) to demonstrate the new usage pattern.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| libsql/src/database/builder.rs | Updated remote_encryption method signatures for RemoteReplica, SyncedDatabase, and Remote; adjusted sync protocol code to apply it conditionally. |
| libsql/examples/encryption_sync.rs | Modified example to construct the builder first and then call .remote_encryption(...) only when needed. |
Comments suppressed due to low confidence (3)
libsql/src/database/builder.rs:309
- Add a unit test to verify that calling
remote_encryptioncorrectly setsinner.remote_encryptiontoSome(encryption_context)for each builder variant.
pub fn remote_encryption(mut self, encryption_context: EncryptionContext) -> Builder<RemoteReplica> {
libsql/examples/encryption_sync.rs:30
- [nitpick] The variable name
encis abbreviated and may be unclear; consider usingencryption_contextorcontextto match the API and improve readability.
if let Some(enc) = encryption {
libsql/src/database/builder.rs:307
- [nitpick] The
remote_encryptionmethod is duplicated across three different builder implementations; consider extracting a shared trait or helper macro to DRY up this logic.
/// Set the encryption context if the database is encrypted in remote server.
This file contains hidden or 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
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.
In builder pattern, it is better to take the
Objectitself instead ofOption<Object>. Sochanged to
This follows the existing practices, and consistent with the code.