-
Notifications
You must be signed in to change notification settings - Fork 533
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
OPFS support for OpenDAL (draft PR) #5269
base: main
Are you sure you want to change the base?
Conversation
@@ -381,6 +381,20 @@ prometheus-client = { version = "0.22.2", optional = true } | |||
tracing = { version = "0.1", optional = true } | |||
# for layers-dtrace | |||
probe = { version = "0.5.1", optional = true } | |||
wasm-bindgen = "0.2.95" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, please mark those dependencies as optional
and hide them under the services-opfs
feature to ensure other builds aren't affected.
access_info | ||
.set_scheme(Scheme::Opfs) | ||
.set_native_capability(Capability { | ||
stat: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please leave them as false
until we actually implement them.
Arc::new(access_info) | ||
} | ||
|
||
async fn create_dir(&self, path: &str, _: OpCreateDir) -> Result<RpCreateDir> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have default implementations for those functions, so we can remove unsupported operations.
impl OpfsCore { | ||
pub async fn store_file(file_name: &str, content: &[u8]) -> Result<(), JsValue> { | ||
// Access the OPFS | ||
let navigator = window().unwrap().navigator(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please try your best to avoid using unwrap()
, and return an error instead.
@LYZJU2019 any updates on this? |
Updates are underway. Stay tuned! |
Hi, perhaps we can divide this work into parts so that community members can participate as well. Considering our recent changes, this PR is almost deprecated, so we need a new one. Maybe we (@LYZJU2019 and @Eason0729) can discuss the detailed tasks in #2442 and collaborate together? |
I have outlined some brief ideas regarding the tasks we need to complete. Feel free to join the discussion: #2442 (comment) |
Which issue does this PR close?
Closes #2442 .
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?