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

rebuild cp_file in GDriveFileSystem to use GoogleDriveFile.Copy #222

Closed

Conversation

simone-viozzi
Copy link
Contributor

@simone-viozzi simone-viozzi commented Aug 9, 2022

Can copy single files and folders, the recursive implementation is in AbstractFileSystem.copy.

Is compatible with AbstractFileSystem.copy so for example:

fs = GDriveFileSystem("root/tmp", auth)

fs.copy('root/tmp/a.pdf', 'root/tmp/folder/b.pdf', recursive=False)
> will copy the file file a.pdf in the folder and rename it to b.pdf

fs.copy('root/tmp/folder1', 'root/tmp/folder/new_folder1', recursive=True)
> will copy the folder folder1 into folder, renaming it new_folder1. will also copy the content 
> of folder1 into new_folder1

There are no tests for copy, but there is one for the move method, that right now is implemented as copy + rm, and the test passes.

todo list:

  • add tests

@shcheklein can you please review the code? Also check if this is backward compatible with the previous implementation.

Thank you.

- it uses `file.Copy` to copy files
- `self._gdrive_create_dir` to copy folders

the recursive implementation is in ffspec
@shcheklein
Copy link
Member

@simone-viozzi hey, thanks. I'll try to find some time this week!

pydrive2/fs/spec.py Outdated Show resolved Hide resolved
pydrive2/fs/spec.py Outdated Show resolved Hide resolved
Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

Good stuff, that's for starting this PR. I've put some comments to improve it and do a bit of research.

pydrive2/fs/spec.py Outdated Show resolved Hide resolved
@simone-viozzi
Copy link
Contributor Author

@shcheklein I noticed that there was no mkdir method and I needed one, so I implemented it.

I also noticed that fs.expand_path only works if the path is one level under self.path. Example:

  • self.path=root/tmp/
  • folder structure like:
    • root/tmp/fo1
    • root/tmp/fo1/file2.pdf
    • root/tmp/fo1/fo2/
    • root/tmp/fo1/fo2/file3.pdf
    • root/tmp/fo1/fo2/fo3/
    • root/tmp/fo1/fo2/fo3/file4.pdf

then:

print(fs.expand_path('root/tmp/fo1', recursive=True))
> ['root/tmp/fo1', 'root/tmp/fo1/file2.pdf', 'root/tmp/fo1/fo2/file3.pdf', 'root/tmp/fo1/fo2/fo3/file4.pdf'] 
# ( correct )

print(fs.expand_path('root/tmp/fo1/fo2/', recursive=True))
> ['root/tmp/fo1/fo2']
# ( wrong! Correct answer is: ['root/tmp/fo1/fo2', 'root/tmp/fo1/fo2/file3.pdf', 'root/tmp/fo1/fo2/fo3/file4.pdf'] )

expand_path use find:

def find(self, path, detail=False, **kwargs):
bucket, base = self.split_path(path)
seen_paths = set()
dir_ids = [self._ids_cache["ids"].copy()]
contents = []
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
}
if not query_ids:
continue
seen_paths |= query_ids.keys()
new_query_ids = {}
dir_ids.append(new_query_ids)
for item in self._gdrive_list_ids(query_ids):
parent_id = item["parents"][0]["id"]
item_path = posixpath.join(query_ids[parent_id], item["title"])
if item["mimeType"] == FOLDER_MIME_TYPE:
new_query_ids[item["id"]] = item_path
self._cache_path_id(item_path, item["id"])
continue
contents.append(
{
"name": posixpath.join(bucket, item_path),
"type": "file",
"size": int(item["fileSize"]),
"checksum": item.get("md5Checksum"),
}
)
if detail:
return {content["name"]: content for content in contents}
else:
return [content["name"] for content in contents]

The same bug happens in find as well.

I lost a good amount of time to understand why copy suddenly stopped working 😅

@simone-viozzi simone-viozzi marked this pull request as ready for review September 2, 2022 16:21
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.

2 participants