-
Notifications
You must be signed in to change notification settings - Fork 100
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
file: support exporting files as a symlink #819
Conversation
Deploying datachain-documentation with Cloudflare Pages
|
if link_type == "symlink": | ||
try: | ||
return self._symlink_to(dst) | ||
except OSError as exc: | ||
if exc.errno not in (errno.ENOTSUP, errno.EXDEV, errno.ENOSYS): | ||
raise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should we do in case of overwrite?
Looks like export
allows you to overwrite files over and over again, but this _symlink_to
will fail with FileExistsError
.
3335d18
to
6e6aa3c
Compare
self.ensure_cached() | ||
source = self.get_local_path() | ||
assert source, "File was not cached" | ||
elif self.source.startswith("file://"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using self.source.startswith("file://")
to avoid creating a filesystem.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #819 +/- ##
==========================================
- Coverage 87.54% 87.47% -0.07%
==========================================
Files 128 128
Lines 11326 11344 +18
Branches 1533 1538 +5
==========================================
+ Hits 9915 9923 +8
- Misses 1026 1033 +7
- Partials 385 388 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
6e6aa3c
to
876d6aa
Compare
876d6aa
to
3136a3b
Compare
assert (tmp_path / "dir" / "myfile.txt").is_symlink() | ||
|
||
dst = Path(file.get_local_path()) if use_cache else path | ||
assert (tmp_path / "dir" / "myfile.txt").resolve() == dst |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using resolve()
instead of readlink()
because WindowsPath.readlink()
returns an extended path that fails to compare (note the //?/
prefix).
-- WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/popen-gw2/default_working_dir0/.datachain/cache/29/89fe8cd7eda0a1e2e28bf837e137f5bdd8d20c506348f67f5671f5a036d529')
++ WindowsPath('//?/C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/popen-gw2/default_working_dir0/.datachain/cache/29/89fe8cd7eda0a1e2e28bf837e137f5bdd8d20c506348f67f5671f5a036d529')
3136a3b
to
11925cb
Compare
@@ -2418,6 +2419,7 @@ def export_files( | |||
signal="file", | |||
placement: FileExportPlacement = "fullpath", | |||
use_cache: bool = True, | |||
link_type: Literal["copy", "symlink"] = "copy", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we update / add docs while we do this (or as a followup)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in 2d9547f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while we on this - any ideas about a better name for the export_file? + it would be great to update docs. Thanks for a quick turnaround @skshetry
I could not think of a good name tbh. So, I left it at that. Some of my suggestions are: |
@@ -2418,6 +2419,7 @@ def export_files( | |||
signal="file", | |||
placement: FileExportPlacement = "fullpath", | |||
use_cache: bool = True, | |||
link_type: Literal["copy", "symlink"] = "copy", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing I forgot to mention - if files fail to symlink, it will always fall back to copy
.
11925cb
to
2d9547f
Compare
Closes #807.
Example Usage: