Skip to content

Conversation

@efiop
Copy link
Contributor

@efiop efiop commented Jun 30, 2021

Fixes #6225

Thank you for the contribution - we'll try to review it as soon as possible. πŸ™

@efiop efiop requested a review from a team as a code owner June 30, 2021 22:46
@efiop efiop requested a review from karajan1001 June 30, 2021 22:46
self.password = ask_password(self.host, self.user, self.port)

def ssh(self, path_info):
def ssh(self):
Copy link
Contributor Author

@efiop efiop Jun 30, 2021

Choose a reason for hiding this comment

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

One issue that arises here is that move() in ObjectDB.add() will no longer work for advanced cases where a user has external-outputs with external cache that is not located on the same filesystem. This very rare though and has been a common problem for other filesystems too in external-output scenario anyway, so I woundn't fix it right now, as that is a hack already that we are keeping https://github.com/iterative/dvc/blob/master/dvc/objects/db/base.py#L78 for regular non-extrnal dvc add and such. We will be adjusting that in save/transfer unification anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, now that this doesn't take any arguments, we could adjust the way it looks and works, but again I would just wait for sshfs migration, since it will simply throw away this code anyway, so doesn't seem like it is worth wasting time refactoring this now.

@efiop efiop changed the title [WIP] ssh: don't use path_info for connections ssh: don't use path_info for connections Jul 1, 2021
@efiop efiop requested review from karajan1001 and removed request for karajan1001 July 1, 2021 11:21
@efiop efiop enabled auto-merge (squash) July 2, 2021 12:14
@efiop efiop requested a review from isidentical July 2, 2021 12:41
@efiop efiop disabled auto-merge July 3, 2021 18:54
@efiop efiop merged commit 07f5f6b into treeverse:master Jul 3, 2021
@efiop efiop added bugfix fixes bug regression Ohh, we broke something :-( labels Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix fixes bug regression Ohh, we broke something :-(

Projects

None yet

Development

Successfully merging this pull request may close these issues.

push: userless ssh url doesn't work

1 participant