feat(dataset): use posix mv semantics when adding with destination #2612
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
m-alisafaee
force-pushed
the
1765-dataset-destination
branch
from
7102147
to
eb004b3
Compare
m-alisafaee
force-pushed
the
1765-dataset-destination
branch
3 times, most recently
from
fdbbb13
to
961bd2f
Compare
m-alisafaee
force-pushed
the
1765-dataset-destination
branch
from
961bd2f
to
22f2ea6
Compare
Panaetius
requested changes
Feb 8, 2022
| """Add a file or directory from a local filesystem.""" | ||
| src = Path(os.path.abspath(path)) | ||
| action = "symlink" if external else "copy" |
There was a problem hiding this comment.
Not something you need to change here, but action seems to be a good candidate for using an Enum
renku/core/management/datasets.py
Outdated
| if multiple_sources and destination_exists and not destination_is_dir: | ||
| raise errors.ParameterError(f"Destination is not a directory: '{destination}'") | ||
|
|
||
| name = path.name if has_multiple_paths or (destination_exists and destination_is_dir) else "" |
There was a problem hiding this comment.
Also with the empty string path join
renku/core/utils/os.py
Outdated
| @@ -59,22 +68,23 @@ def get_relative_paths(base: Union[Path, str], paths: List[Union[Path, str]]) -> | |||
| return relative_paths | |||
|
|
|||
|
|
|||
| def get_files_recursively(path: Path) -> Generator[Path, None, None]: | |||
There was a problem hiding this comment.
Not sure this is an issue, but the method is called get_files_recursively but then the body is not actually recursive. Should this be
if not subpath.is_dir():
yield subpath
else:
yield from get_files_recursively(subpath)
?
There was a problem hiding this comment.
I renamed it to get_files
Panaetius
approved these changes
Feb 17, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adding data to a dataset with passing a destination using
--destinationflag, acts similar to posixcpandmvcommands: Added file is renamed it destination doesn't exists or it's a file and it's copied to the destination otherwise.Fixes #1765
Examples: