Skip to content
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

feat(ofs): introduce ofs execute bin #4033

Merged
merged 7 commits into from
Jan 20, 2024
Merged

feat(ofs): introduce ofs execute bin #4033

merged 7 commits into from
Jan 20, 2024

Conversation

oowl
Copy link
Member

@oowl oowl commented Jan 20, 2024

part of #3782

@oowl oowl requested review from Xuanwo and PsiACE as code owners January 20, 2024 08:59
@oowl oowl marked this pull request as draft January 20, 2024 08:59
@github-actions github-actions bot requested a review from xyjixyjixyji January 20, 2024 08:59
bin/ofs/ofs.toml.example Outdated Show resolved Hide resolved
@oowl
Copy link
Member Author

oowl commented Jan 20, 2024

Now the ofs command running information as below

╰─$ ./target/debug/ofs --help
OpenDAL File System

Usage: ofs --mount-path <MOUNT_PATH> --backend <BACKEND>

Options:
  -m, --mount-path <MOUNT_PATH>  fuse mount path [env: OFS_MOUNT_PATH=]
  -b, --backend <BACKEND>        location of opendal service format: <scheme>://<host>:<port> example: s3://127.0.0.1:9000?access_key=xxx&secret_key=xxx [env: OFS_BACKEND=]
  -h, --help                     Print help
  -V, --version                  Print version
╭─ouyang@owl-home ~/code/opendal ‹feat/fuse●› 
╰─$ RUST_LOG=debug ./target/debug/ofs -b "fs://?root=/tmp" -m ./example  
[2024-01-20T12:22:45Z DEBUG opendal::services::fs::backend] backend build started: FsBuilder { root: Some("/tmp"), atomic_write_dir: None }
[2024-01-20T12:22:45Z DEBUG opendal::services::fs::backend] backend use root /tmp
[2024-01-20T12:22:45Z DEBUG opendal::services::fs::backend] backend build finished: FsBuilder { root: None, atomic_write_dir: None }
[2024-01-20T12:22:48Z DEBUG ofs] getattr(path=Some("/"))
[2024-01-20T12:22:48Z DEBUG ofs] getattr(path=Some("/"))
[2024-01-20T12:22:53Z DEBUG ofs] getattr(path=Some("/"))
[2024-01-20T12:22:53Z DEBUG ofs] getattr(path=Some("/"))
[2024-01-20T12:22:53Z DEBUG ofs] getattr(path=Some("/"))
[2024-01-20T12:22:53Z DEBUG ofs] getattr(path=Some("/"))
[2024-01-20T12:23:09Z DEBUG ofs] getattr(path=Some("/"))
[2024-01-20T12:23:09Z DEBUG ofs] getattr(path=Some("/"))

@oowl oowl marked this pull request as ready for review January 20, 2024 12:25
@oowl
Copy link
Member Author

oowl commented Jan 20, 2024

I will do another part about fuse implementation detail in the next PR.

@oowl oowl requested a review from Xuanwo January 20, 2024 12:44
bin/ofs/Cargo.toml Outdated Show resolved Hide resolved
@oowl oowl requested a review from suyanhanx January 20, 2024 16:10
@Xuanwo
Copy link
Member

Xuanwo commented Jan 20, 2024

Now the ofs command running information as below

Comparing

ofs --mount-path /path/to/mount/path --backend "fs://?root=/tmp"

I prefer to have

ofs s3://bucket/root/path /mount/path

The required values are the backend path and mount path. Using positional arguments makes more sense to me.

@suyanhanx
Copy link
Member

We may add some commands, like mount or unmount.
It won't provide only one function.

@Xuanwo
Copy link
Member

Xuanwo commented Jan 20, 2024

We may add some commands, like mount or unmount.

Exit ofs should unmount the fs. We don't need to provide more commands for this application.

@oowl
Copy link
Member Author

oowl commented Jan 20, 2024

Now the ofs command as below

╰─$ ./target/debug/ofs --help      
Error: parse command line arguments

Caused by:
    OpenDAL File System
    
    Usage: ofs <MOUNT_PATH> <BACKEND>
    
    Arguments:
      <MOUNT_PATH>  fuse mount path [env: OFS_MOUNT_PATH=]
      <BACKEND>     location of opendal service format: <scheme>://?<key>=<value>&<key>=<value> example: fs://root=/tmp [env: OFS_BACKEND=]
    
    Options:
      -h, --help     Print help
      -V, --version  Print version

╰─$ RUST_LOG=debug ./target/debug/ofs ./exampless "fs://?root=/tmp"     
[2024-01-20T16:47:07Z DEBUG opendal::services::fs::backend] backend build started: FsBuilder { root: Some("/tmp"), atomic_write_dir: None }
[2024-01-20T16:47:07Z DEBUG opendal::services::fs::backend] backend use root /tmp
[2024-01-20T16:47:07Z DEBUG opendal::services::fs::backend] backend build finished: FsBuilder { root: None, atomic_write_dir: None }
[2024-01-20T16:47:09Z DEBUG ofs] getattr(path=Some("/"))

cc @Xuanwo

@Xuanwo
Copy link
Member

Xuanwo commented Jan 20, 2024

Thank you! I have additional ideas for the UX of ofs, but I believe this PR is ready to be merged as is.

@oowl
Copy link
Member Author

oowl commented Jan 20, 2024

Thank you! I have additional ideas for the UX of ofs, but I believe this PR is ready to be merged as is.

We can talk about this in another issue.

@Xuanwo Xuanwo merged commit 6d3b4a2 into main Jan 20, 2024
22 checks passed
@Xuanwo Xuanwo deleted the feat/fuse branch January 20, 2024 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants