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

GDriveFileSystem: Raise FIleNotFoundError on ls. #283

Merged
merged 5 commits into from
Jun 22, 2023
Merged

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented Jun 15, 2023

@daavoo daavoo self-assigned this Jun 15, 2023
@daavoo daavoo temporarily deployed to internal June 15, 2023 11:09 — with GitHub Actions Inactive
@daavoo daavoo requested a review from a team June 15, 2023 11:09
pydrive2/fs/spec.py Outdated Show resolved Hide resolved
@daavoo daavoo temporarily deployed to internal June 15, 2023 11:17 — with GitHub Actions Inactive
@daavoo daavoo temporarily deployed to internal June 15, 2023 11:19 — with GitHub Actions Inactive
@efiop efiop temporarily deployed to internal June 15, 2023 12:27 — with GitHub Actions Inactive
@skshetry skshetry temporarily deployed to internal June 16, 2023 06:08 — with GitHub Actions Inactive
@skshetry skshetry temporarily deployed to internal June 16, 2023 06:10 — with GitHub Actions Inactive
Comment on lines +120 to +122
_, base = fs.split_path(remote_dir + "dir/")
fs._path_to_item_ids(base, create=True)
assert fs.ls(remote_dir + "dir/") == []
Copy link
Member

Choose a reason for hiding this comment

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

Turns out, fs.mkdir() is not implemented in GDriveFileSystem. I was not sure if it was needed as it has been working fine for so long. So I only fixed in test to create a directory.

@skshetry skshetry requested a review from efiop June 16, 2023 06:14
@skshetry
Copy link
Member

Investigating some failures in iterative/dvc-gdrive#28. Will merge this after they turn green.

@skshetry
Copy link
Member

Looks like find() is also not working correctly. Will look into it next week.

@daavoo
Copy link
Contributor Author

daavoo commented Jun 16, 2023

Looks like find() is also not working correctly. Will look into it next week.

What is the failure or case where is not working?

@skshetry
Copy link
Member

Looks like find() is also not working correctly. Will look into it next week.

What is the failure or case where is not working?

See iterative/dvc-gdrive#28. The following returns an empty list even though the file exists, failing status checks.

>>> fs.find("root/<>/files/md5")
[]

I am still trying to understand internals of GDriveFileSystem on how to fix it.

This piece of code is problematic:

while dir_ids:
query_ids = {
dir_id: dir_name
for dir_id, dir_name in dir_ids.pop().items()
if posixpath.commonpath([base, dir_name]) == base
if dir_id not in seen_paths
}

@skshetry skshetry temporarily deployed to internal June 22, 2023 13:22 — with GitHub Actions Inactive
@skshetry skshetry merged commit 37551b3 into main Jun 22, 2023
@skshetry skshetry deleted the dvc-issue-9607 branch June 22, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants