-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
dvcfs: handle NotADirectoryError from datafs.ls #9746
Conversation
Any idea for where to test this? |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #9746 +/- ##
==========================================
- Coverage 90.48% 90.47% -0.01%
==========================================
Files 480 480
Lines 36528 36534 +6
Branches 5252 5252
==========================================
+ Hits 33051 33053 +2
- Misses 2885 2887 +2
- Partials 592 594 +2
☔ View full report in Codecov by Sentry. |
For the record: I'll add a test, no worries. Thank you! 🙏 |
def test_ls_dirty(tmp_dir, dvc): | ||
tmp_dir.dvc_gen({"data": "data"}) | ||
(tmp_dir / "data").unlink() | ||
|
||
tmp_dir.gen({"data": {"foo": "foo", "bar": "bar"}}) | ||
|
||
fs = DVCFileSystem(repo=dvc) | ||
assert set(fs.ls("data")) == {"data/foo", "data/bar"} |
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.
This is a little different from what's described in the issue (because there is dvc.lock there), but it is the same situation of having a dirty workspace compared to what is recorded (or missing like with missing dvc.lock) in dvc files.
EDIT: fixed in iterative/dvc-s3#46 |
Before
dvc_fs.ls(path)
withoutdvc.lock
would returnpath/path
. Now it won't return anything sincedvc_fs
has no info about what's in that path.Fixes #9745