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

DM-24352: Add new auto and link transfer modes #252

Merged
merged 3 commits into from Apr 6, 2020
Merged

Conversation

timj
Copy link
Member

@timj timj commented Apr 4, 2020

  • Auto selects the default based on context
  • link tries hardlink and falls back to symlink

@DinoBektesevic in theory with this ticket you should be able to do a 2 to 3 conversion directly to S3.

timj added 2 commits April 4, 2020 15:33
The behavior of auto depends on the datastore.
auto will now fall back to this mode in posix datastore.
@timj timj requested a review from TallJimbo April 4, 2020 22:49
return transfer
try:
# Assume first dataset is representative
self._pathInStore(datasets[0].path)
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it could yield pretty surprising behavior if the assumption is incorrect. Better to check them all and disallow "auto" if the answer is inconsistent, even if that could be a lot of overhead. After all, it's always possible to avoid that overhead by being explicit about the transfer mode.

An alternative would be to just make in-place the default so it's an error if the file is outside. I'm slightly concerned that having symlinks be any kind of default (even the "default of last resort") could be bad, because it makes for a fragile repository.

Raises
------
RuntimeError
Raised if the supplied path is outside the datastore.
Copy link
Member

Choose a reason for hiding this comment

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

Given that in at least one context this is a totally normal condition (as opposed to an error) it might be better to return None.

@timj timj merged commit 396cc6f into master Apr 6, 2020
@timj timj deleted the tickets/DM-24352 branch April 6, 2020 20:16
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.

None yet

2 participants