Xuanwo commented on code in PR #3119: URL: https://github.com/apache/incubator-opendal/pull/3119#discussion_r1328680742
########## integrations/README.md: ########## @@ -5,3 +5,5 @@ This folder contains the integrations for OpenDAL. Integrations are used to inte ## Available Integrations - [`object_store_opendal`](./object_store): Use OpenDAL as a backend for the [object_store](https://docs.rs/object_store/latest/object_store/). + +- [`dav-server-fs-opendal`](./dav-server-fs-opendal/): Use OpenDAL as a backend to access data in various service with WebDAV protocol. Review Comment: This crate's name is `dav-server-opendalfs` (follow the upstream pattern), but the folder is `dav-server` (since it's in our project). ########## integrations/dav-server-fs-opendal/.gitignore: ########## @@ -0,0 +1 @@ +Cargo.lock Review Comment: Please don't ignore `Cargo.lock` file. ########## integrations/dav-server-fs-opendal/src/lib.rs: ########## @@ -0,0 +1,6 @@ +mod webdav_dir_entry; +mod webdav_file; +mod webdav_metadata; +pub mod webdavfs; + +pub use dav_server::*; Review Comment: Please don't `pub use` an external lib. ########## integrations/dav-server-fs-opendal/tests/test.rs: ########## @@ -0,0 +1,38 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use anyhow::Result; +use dav_server_fs_opendal::webdavfs::WebdavFs; +use dav_server_fs_opendal::davpath::DavPath; +use dav_server_fs_opendal::fs::DavFileSystem; +use opendal::services::Fs; +use opendal::Operator; + +#[tokio::test] +async fn test() -> Result<()> { Review Comment: Good start! We can extand it in the future. ########## integrations/dav-server-fs-opendal/src/webdav_dir_entry.rs: ########## @@ -0,0 +1,54 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use dav_server::fs::DavDirEntry; +use futures::FutureExt; +use opendal::Entry; +use opendal::Operator; + +use super::webdav_file::convert_error; +use super::webdav_metadata::WebdavMetaData; + +pub struct WebDAVDirEntry { Review Comment: All public types in crate must have comments. ########## integrations/dav-server-fs-opendal/Cargo.toml: ########## @@ -0,0 +1,58 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +[package] +description = "Use OpenDAL as a backend to access data in various service with WebDAV protocol" +name = "dav-server-fs-opendal" + +authors.workspace = true +edition.workspace = true +homepage.workspace = true +license.workspace = true +repository.workspace = true +rust-version.workspace = true +version.workspace = true + +[dependencies] +anyhow = "1" +axum = "0.6" +chrono = "0.4.28" +clap = { version = "4", features = ["cargo", "string"] } +dirs = "5.0.0" +# bytes = { version = "1.4.0", optional = true } +# dav-server = { version = "0.5.5", optional = true } Review Comment: Remove not used code ########## integrations/dav-server-fs-opendal/src/webdav_dir_entry.rs: ########## Review Comment: Maybe we can remove the `webdav_` prefix? ########## integrations/dav-server-fs-opendal/src/lib.rs: ########## @@ -0,0 +1,6 @@ +mod webdav_dir_entry; +mod webdav_file; +mod webdav_metadata; +pub mod webdavfs; Review Comment: How about expose an `OpendalFs` like [`LocalFs`](https://docs.rs/dav-server/latest/dav_server/localfs/index.html)? ########## integrations/dav-server-fs-opendal/README.md: ########## @@ -0,0 +1,3 @@ +# dav-server-fs-opendal + +todo Review Comment: Please replace `todo` with a simple sentence. ########## integrations/dav-server-fs-opendal/src/webdav_file.rs: ########## @@ -0,0 +1,94 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::io::SeekFrom; + +use bytes::Bytes; +use dav_server::davpath::DavPath; +use dav_server::fs::DavFile; +use dav_server::fs::DavMetaData; +use dav_server::fs::FsFuture; +use dav_server::fs::OpenOptions; +use futures::FutureExt; +use opendal::Operator; + +use super::webdav_metadata::WebdavMetaData; + +#[derive(Debug)] +pub struct WebdavFile { + pub op: Operator, Review Comment: Do we want those fields to be public? ########## integrations/dav-server-fs-opendal/Cargo.toml: ########## @@ -0,0 +1,58 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +[package] +description = "Use OpenDAL as a backend to access data in various service with WebDAV protocol" +name = "dav-server-fs-opendal" + +authors.workspace = true +edition.workspace = true +homepage.workspace = true +license.workspace = true +repository.workspace = true +rust-version.workspace = true +version.workspace = true + +[dependencies] +anyhow = "1" +axum = "0.6" +chrono = "0.4.28" +clap = { version = "4", features = ["cargo", "string"] } Review Comment: Maybe we don't need clap anymore? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
