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

[#6320] feat (gvfs-fuse): Support mount and umount command for gvfs-fuse command line tools #6321

Merged
merged 11 commits into from
Feb 7, 2025

Conversation

diqiu50
Copy link
Contributor

@diqiu50 diqiu50 commented Jan 17, 2025

What changes were proposed in this pull request?

  1. Support mount and umount command
  2. Make mount create a daemon process to run background

Why are the changes needed?

Fix: #6320

Does this PR introduce any user-facing change?

No

How was this patch tested?

IT

@diqiu50 diqiu50 changed the title [#6320] feat (gvfs-fuse): Support mount and umount command for gvfs-fuse [#6320] feat (gvfs-fuse): Support mount and umount command for gvfs-fuse command line tools Jan 17, 2025
@diqiu50 diqiu50 self-assigned this Jan 17, 2025
clients/filesystem-fuse/src/command_args.rs Outdated Show resolved Hide resolved
config: Option<String>,

#[arg(short, long, help = "Debug level", default_value_t = 0)]
debug: u8,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is an overdesign?
Are we really using this argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we will need to use this argument in PR #5905.

@jerryshao jerryshao requested a review from FANNG1 January 22, 2025 11:21
mount_point: String,

#[arg(help = "The URI of the GVFS fileset")]
location: String,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The meaning of location and config seems not clear

Copy link
Contributor Author

@diqiu50 diqiu50 Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change location to fileset_location
Using "config" as a configuration name is very common.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you provide an example of fileset_location and add the example to the help message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix, add an example

Copy link
Contributor Author

@diqiu50 diqiu50 Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

help message like:

./gvfs-fuse mount --help
Usage: gvfs-fuse mount [OPTIONS] <MOUNT_POINT> <FILESET_LOCATION>

Arguments:
  <MOUNT_POINT>       Mount point for the filesystem
  <FILESET_LOCATION>  The URI of the GVFS fileset, like gvfs://fileset/my_catalog/my_schema/my_fileset

Options:
  -c, --config <CONFIG>  Path to the configuration file
  -d, --debug <DEBUG>    Debug level [default: 0]
  -f, --foreground       Run in foreground
  -h, --help             Print help

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isFILESET_URI more meaningful than FILESET_LOCATION?

@diqiu50 diqiu50 requested a review from FANNG1 February 5, 2025 09:39
@jerryshao jerryshao merged commit e85e47d into apache:main Feb 7, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Subtask] Gvfs Fuse command line Tools: support mount and umount command
4 participants