Skip to content

Conversation

@99-NinetyNine
Copy link
Contributor

No description provided.

install_requires =
fsspec>=2021.8.1
asyncssh>=2.11.0,<3
asyncssh==2.19.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep it this way:

Suggested change
asyncssh==2.19.0
asyncssh>=2.11.0,<3


if local_user is None:
with suppress(KeyError):
with suppress(OSError): # Use OSError as getuser() might raise this.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep KeyError too. I see that it raises OSError now in 3.13, but that is not true for older versions.

Suggested change
with suppress(OSError): # Use OSError as getuser() might raise this.
with suppress(OSError): # Use OSError as getuser() might raise this.

return SSHClientConfig.load(
# Check asyncssh version
version = tuple(map(int, asyncssh.__version__.split(".")))
if version <= (2, 18, 0): # Compare version properly
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if version <= (2, 18, 0): # Compare version properly
if version < (2, 19, 0): # Compare version properly

Comment on lines +40 to +41
canonical, # Use correct parameter
final, # Use correct parameter
Copy link
Collaborator

Choose a reason for hiding this comment

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

All these comments are unnecessary.

def parse_config(
*, host, user=(), port=(), local_user=None, config_files=None
):
def parse_config(*, host, user=(), port=(), local_user=None, config_files=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd also add a warning that the function is deprecated.

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.

2 participants