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.

Sign up for GitHub

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

Draft: feat: alias support prefix #442

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Draft: feat: alias support prefix #442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions megfile/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,18 +734,26 @@
help="alias config file, default is $HOME/.config/megfile/aliases.conf",
)
@click.argument("name")
@click.argument("protocol")
@click.argument("protocol_or_path")
@click.option("--no-cover", is_flag=True, help="Not cover the same-name config")
def alias(path, name, protocol, no_cover):
def alias(path, name, protocol_or_path, no_cover):
path = os.path.expanduser(path)
config = configparser.ConfigParser()
if os.path.exists(path):
config.read(path)
if name in config.sections() and no_cover:
raise NameError(f"alias-name has been used: {name}")
config[name] = {
"protocol": protocol,
}

if "://" in protocol_or_path:
protocol, prefix = protocol_or_path.split("://", maxsplit=1)
config[name] = {

Check warning on line 749 in megfile/cli.py

View check run for this annotation

Codecov / codecov/patch

megfile/cli.py#L748-L749 

Added lines #L748 - L749 were not covered by tests
"protocol": protocol,
"prefix": prefix,
}
else:
config[name] = {
"protocol": protocol_or_path,
}

_safe_makedirs(os.path.dirname(path)) # make sure dirpath exist
with open(path, "w") as fp:
Expand Down
3 changes: 2 additions & 1 deletion megfile/smart_path.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,9 @@ def _extract_protocol(
raise ProtocolNotFoundError("protocol not found: %r" % path)
aliases: Dict[str, Dict[str, str]] = cls._aliases # pyre-ignore[9]
if protocol in aliases:
prefix = aliases[protocol].get("prefix", "")
protocol = aliases[protocol]["protocol"]
path = "%s://%s" % (protocol, path_without_protocol)
path = "%s://%s%s" % (protocol, prefix, path_without_protocol)
Copy link
Collaborator

Choose a reason for hiding this comment

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

感觉这个 prefix 可能会引发客服问题，用户可能不知道后面留几个斜杠，不同的协议可能还不一样，比如有人填的是 sftp://ubuntu@host ，正确的应该填 sftp://ubuntu@host// 吧，还有相对路径和绝对路径不知道有没有影响，🤣

Copy link
Member Author

@bbtfr bbtfr Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

我自己写也容易写错，但感觉这个并不是 alias 引发的，是 sftp 咱可能定义的太奇怪了
偷偷帮用户补一个 / 感觉也容易有坑，所以我就 bypass 直接传了

你觉得这里应该用 join 吗？比如没写 / 自动补一个？

  • sftp://ubuntu@host = sftp://ubuntu@host/
  • sftp://ubuntu@host/ = sftp://ubuntu@host/
  • sftp://ubuntu@host// = sftp://ubuntu@host//

Copy link
Collaborator

Choose a reason for hiding this comment

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

搞个 sftp config 文件会不会好点

Copy link
Member Author

Choose a reason for hiding this comment

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

先标成 draft 吧，再想想咋整

return protocol, path

@classmethod
Expand Down
14 changes: 12 additions & 2 deletions tests/test_smart_path.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
from megfile.smart_path import PurePath, SmartPath, _load_aliases_config, aliases_config
from megfile.stdio_path import StdioPath

from .test_sftp import sftp_mocker # noqa: F401

FS_PROTOCOL_PREFIX = FSPath.protocol + "://"
FS_TEST_ABSOLUTE_PATH = "/test/dir/file"
FS_TEST_ABSOLUTE_PATH_WITH_PROTOCOL = FS_PROTOCOL_PREFIX + FS_TEST_ABSOLUTE_PATH
Expand Down Expand Up @@ -76,7 +78,7 @@ def test_register_result():
assert SmartPath.from_uri(FS_TEST_ABSOLUTE_PATH) == SmartPath(FS_TEST_ABSOLUTE_PATH)


def test_aliases(fs):
def test_aliases(fs, sftp_mocker):
config_path = os.path.expanduser(aliases_config)
fs.create_file(
config_path,
Expand All @@ -88,10 +90,18 @@ def test_aliases(fs):
with patch.object(SmartPath, "_aliases", new_callable=PropertyMock) as mock_aliases:
mock_aliases.return_value = aliases
assert (
SmartPath.from_uri("oss2://bucket/dir/file").pathlike
SmartPath("oss2://bucket/dir/file").pathlike
== SmartPath("s3+oss2://bucket/dir/file").pathlike
)

aliases = {"dev": {"protocol": "sftp", "prefix": "ubuntu@host//"}}
with patch.object(SmartPath, "_aliases", new_callable=PropertyMock) as mock_aliases:
mock_aliases.return_value = aliases
assert (
SmartPath("dev://dir/file").pathlike
== SmartPath("sftp://ubuntu@host//dir/file").pathlike
)


@patch.object(SmartPath, "_create_pathlike")
def test_init(funcA):
Expand Down
Loading