This repository has been archived by the owner on Sep 21, 2024. It is now read-only.
chore: Genercize storage across gateway and rename "default" storages #622
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
jsantell
commented
Sep 5, 2023
| pub async fn to_storage(&self) -> Result<NativeStorage> { | ||
| NativeStorage::new(NativeStorageInit::Path(PathBuf::from(self))) | ||
| pub async fn to_storage(&self) -> Result<SledStorage> { | ||
| SledStorage::new(SledStorageInit::Path(PathBuf::from(self))) |
There was a problem hiding this comment.
This (and make_disposable_store) are the last non-generic storage invocations AFAICT. As PlatformStorage could be a non-primitive (like IpfsStorage wrapper), some options:
Storageadds anew_from_pathmethod that works on all Storage impls (ornew_from_str)noosphere::platformexposes something likemake_platform_storage(&str), though we want to distinguishPlatformStorage(which may be wrapped by a non-primitive store) from the underlying storage primitive, though currently this would involve a lot of duplicate defs if we want to swap around backends during testing, currently apple/aarch64 and non-native-apple multiplied by storage backends
…orage. Make storage generic over gateway.
jsantell
force-pushed
the
generic-storage
branch
from
d082df2 to
5bb254b
Compare
|
Gotcha. It's the storage for the "current platform" where in that's set
with feature flags.
…On Tue, Sep 5, 2023, 4:42 PM Jordan Santell ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In rust/noosphere/src/platform.rs
<#622 (comment)>
:
> @@ -19,13 +19,13 @@ mod inner {
pub type PlatformKeyStorage = InsecureKeyStorage;
#[cfg(not(feature = "ipfs-storage"))]
- pub type PlatformStorage = NativeStorage;
+ pub type PlatformStorage = SledStorage;
PlatformStorage here is the default storage used for a given platform for
(mostly?) sphere storage, where its (given this patch) IndexedDbStorage
(previously WebStorage) on wasm32 builds, and SledStorage (previously
NativeStorage) elsewhere (though apple/aarch64 builds here are distinct,
mostly paving the way for apple-specific key enclaves) -- this is where
we'd compose different backends for different platforms e.g. maybe
SledStorage for desktop builds but RocksDbStorage on iOS
—
Reply to this email directly, view it on GitHub
<#622 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAA6DMMARL5NPSJKUUVHFTXY62FZANCNFSM6AAAAAA4MGOWC4>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
justinabrahms
approved these changes
Sep 6, 2023
Comment on lines
+61
to
+62
| St: Send + Sync, | ||
| S: Storage + 'static, |
There was a problem hiding this comment.
Just noting that there is no t (you named it St) in Send + Sync and there are three in Storage + 'static, which will probably confuse people w/ naming.
There was a problem hiding this comment.
Nice catch! The St type reflects the underlying axum State as rationale for naming, but you make a great point, will update to something less ambiguous
There was a problem hiding this comment.
I personally wouldn't balk at a non-abbreviated identifier for the generic type.
|
Closing in lieu of superceding work in #623 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
chore: Rename NativeStorage -> SledStorage, WebStorage -> IndexedDbStorage. Make storage generic over gateway. In preparation for exploring pluggable backends in #607