You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "zjregee (via GitHub)" <gi...@apache.org> on 2024/06/04 06:14:43 UTC
[PR] feat(ovfs): add readme and user configuration [opendal]
zjregee opened a new pull request, #4686:
URL: https://github.com/apache/opendal/pull/4686
(no comment)
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ovfs): add readme and user configuration [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4686:
URL: https://github.com/apache/opendal/pull/4686#discussion_r1636152138
##########
bin/ovfs/src/main.rs:
##########
@@ -19,6 +19,59 @@ mod error;
mod virtiofs;
mod virtiofs_utils;
+use std::collections::HashMap;
+use std::str::FromStr;
+
+use clap::Parser;
+use opendal::{Operator, Scheme};
+use url::Url;
+
+/// User configuration for ovfs.
+#[derive(Parser, Debug)]
+#[command(version, about)]
+struct Config {
+ /// socket path used to connect with VMs
+ #[arg(env = "OVFS_MOUT_PATH", index = 1)]
+ socket_path: String,
+
+ /// location of opendal service
+ /// format: <scheme>://?<key>=<value>&<key>=<value>
+ /// example: fs://?root=/tmp
+ #[arg(env = "OVFS_BACKEND", index = 2)]
+ backend: Url,
Review Comment:
I will initiate another discussion with ofs. However, I believe ovfs is a separate project that shouldn't adhere to the same design as ofs, especially when that design seems flawed.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ovfs): add readme and user configuration [opendal]
Posted by "zjregee (via GitHub)" <gi...@apache.org>.
zjregee commented on code in PR #4686:
URL: https://github.com/apache/opendal/pull/4686#discussion_r1636144597
##########
bin/ovfs/src/main.rs:
##########
@@ -19,6 +19,59 @@ mod error;
mod virtiofs;
mod virtiofs_utils;
+use std::collections::HashMap;
+use std::str::FromStr;
+
+use clap::Parser;
+use opendal::{Operator, Scheme};
+use url::Url;
+
+/// User configuration for ovfs.
+#[derive(Parser, Debug)]
+#[command(version, about)]
+struct Config {
+ /// socket path used to connect with VMs
+ #[arg(env = "OVFS_MOUT_PATH", index = 1)]
+ socket_path: String,
+
+ /// location of opendal service
+ /// format: <scheme>://?<key>=<value>&<key>=<value>
+ /// example: fs://?root=/tmp
+ #[arg(env = "OVFS_BACKEND", index = 2)]
+ backend: Url,
Review Comment:
This is ok for me, but I will have a concern about the alignment with ofs. I think ofs and ovfs should be used in a similar way. The usage here refers to the current implementation of ofs.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ovfs): add readme and user configuration [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4686:
URL: https://github.com/apache/opendal/pull/4686#discussion_r1636152138
##########
bin/ovfs/src/main.rs:
##########
@@ -19,6 +19,59 @@ mod error;
mod virtiofs;
mod virtiofs_utils;
+use std::collections::HashMap;
+use std::str::FromStr;
+
+use clap::Parser;
+use opendal::{Operator, Scheme};
+use url::Url;
+
+/// User configuration for ovfs.
+#[derive(Parser, Debug)]
+#[command(version, about)]
+struct Config {
+ /// socket path used to connect with VMs
+ #[arg(env = "OVFS_MOUT_PATH", index = 1)]
+ socket_path: String,
+
+ /// location of opendal service
+ /// format: <scheme>://?<key>=<value>&<key>=<value>
+ /// example: fs://?root=/tmp
+ #[arg(env = "OVFS_BACKEND", index = 2)]
+ backend: Url,
Review Comment:
I will initiate another discussion with ofs. However, I believe ovfs is a separate project that shouldn't adhere to the same design as ofs, especially when that design seems flawed.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ovfs): add readme and user configuration [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4686:
URL: https://github.com/apache/opendal/pull/4686#discussion_r1636135185
##########
bin/ovfs/src/main.rs:
##########
@@ -19,6 +19,59 @@ mod error;
mod virtiofs;
mod virtiofs_utils;
+use std::collections::HashMap;
+use std::str::FromStr;
+
+use clap::Parser;
+use opendal::{Operator, Scheme};
+use url::Url;
+
+/// User configuration for ovfs.
+#[derive(Parser, Debug)]
+#[command(version, about)]
+struct Config {
+ /// socket path used to connect with VMs
+ #[arg(env = "OVFS_MOUT_PATH", index = 1)]
+ socket_path: String,
+
+ /// location of opendal service
+ /// format: <scheme>://?<key>=<value>&<key>=<value>
+ /// example: fs://?root=/tmp
+ #[arg(env = "OVFS_BACKEND", index = 2)]
+ backend: Url,
Review Comment:
I don't like the idea of introducing a URL here. How about just using a `HashMap`? Our command line coule be like:
```shell
ovfs path/to/mount --scheme=s3 --access-key-id=xxx --secret-access-key=yyy --root=/tmp
```
Reading `OVFS_BACKEND` also doesn't make sense to me. We can start by accepting a simple map.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ovfs): add readme and user configuration [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #4686:
URL: https://github.com/apache/opendal/pull/4686#issuecomment-2783039630
Thank you @zjregee for working on this. Feel free to open a discussion about the futures of ovfs project.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ovfs): add readme and user configuration [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo closed pull request #4686: feat(ovfs): add readme and user configuration
URL: https://github.com/apache/opendal/pull/4686
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ovfs): add readme and user configuration [opendal]
Posted by "zjregee (via GitHub)" <gi...@apache.org>.
zjregee commented on code in PR #4686:
URL: https://github.com/apache/opendal/pull/4686#discussion_r1637495754
##########
bin/ovfs/src/main.rs:
##########
@@ -19,6 +19,59 @@ mod error;
mod virtiofs;
mod virtiofs_utils;
+use std::collections::HashMap;
+use std::str::FromStr;
+
+use clap::Parser;
+use opendal::{Operator, Scheme};
+use url::Url;
+
+/// User configuration for ovfs.
+#[derive(Parser, Debug)]
+#[command(version, about)]
+struct Config {
+ /// socket path used to connect with VMs
+ #[arg(env = "OVFS_MOUT_PATH", index = 1)]
+ socket_path: String,
+
+ /// location of opendal service
+ /// format: <scheme>://?<key>=<value>&<key>=<value>
+ /// example: fs://?root=/tmp
+ #[arg(env = "OVFS_BACKEND", index = 2)]
+ backend: Url,
Review Comment:
Got this. I'll draft this PR first. We should reopen this PR to discuss the user interface of the binary after the ovfs library is fully developed.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ovfs): add readme and user configuration [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4686:
URL: https://github.com/apache/opendal/pull/4686#discussion_r1636161728
##########
bin/ovfs/src/main.rs:
##########
@@ -19,6 +19,59 @@ mod error;
mod virtiofs;
mod virtiofs_utils;
+use std::collections::HashMap;
+use std::str::FromStr;
+
+use clap::Parser;
+use opendal::{Operator, Scheme};
+use url::Url;
+
+/// User configuration for ovfs.
+#[derive(Parser, Debug)]
+#[command(version, about)]
+struct Config {
+ /// socket path used to connect with VMs
+ #[arg(env = "OVFS_MOUT_PATH", index = 1)]
+ socket_path: String,
+
+ /// location of opendal service
+ /// format: <scheme>://?<key>=<value>&<key>=<value>
+ /// example: fs://?root=/tmp
+ #[arg(env = "OVFS_BACKEND", index = 2)]
+ backend: Url,
Review Comment:
I feel that we're currently pushing too aggressively to release the binary to users. Instead, we should focus on developing a library, ensuring it functions correctly and passes the tests before making it available to users.
For example, we don't need to play with the config/command line/args right now. We should make it work and than design our user interface.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ovfs): add readme and user configuration [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4686:
URL: https://github.com/apache/opendal/pull/4686#discussion_r1636135185
##########
bin/ovfs/src/main.rs:
##########
@@ -19,6 +19,59 @@ mod error;
mod virtiofs;
mod virtiofs_utils;
+use std::collections::HashMap;
+use std::str::FromStr;
+
+use clap::Parser;
+use opendal::{Operator, Scheme};
+use url::Url;
+
+/// User configuration for ovfs.
+#[derive(Parser, Debug)]
+#[command(version, about)]
+struct Config {
+ /// socket path used to connect with VMs
+ #[arg(env = "OVFS_MOUT_PATH", index = 1)]
+ socket_path: String,
+
+ /// location of opendal service
+ /// format: <scheme>://?<key>=<value>&<key>=<value>
+ /// example: fs://?root=/tmp
+ #[arg(env = "OVFS_BACKEND", index = 2)]
+ backend: Url,
Review Comment:
I don't like the idea of introducing a URL here. How about just using a `HashMap`? Our command line coule be like:
```shell
ovfs path/to/mount --scheme=s3 --access-key-id=xxx --secret-access-key=yyy --root=/tmp
```
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org