search

iterative / PyDrive2

Google Drive API Python wrapper library. Maintained fork of PyDrive.
https://docs.iterative.ai/PyDrive2
Other
581 stars 69 forks source link

make checksum optional in GDriveFileSystem #195

Closed simone-viozzi closed 2 years ago

simone-viozzi commented 2 years ago

As discussed in #191, to make GDriveFileSystem compatible with google docs / sheet files the checksum in the metadata need to be optional.

I tried to run the tests in test_fs.py and it's all green.

shcheklein commented 2 years ago

@efiop could someone familiar with DVC / fsspec check this please? That it does't break anything downstream (especially something from the fsspec perspective?). That would be really helpful.

@simone-viozzi thanks for the PR! looks like you need to run linters / pre-commit. Checks are failing.

efiop commented 2 years ago

@efiop could someone familiar with DVC / fsspec check this please? That it does't break anything downstream (especially something from the fsspec perspective?). That would be really helpful.

Using None is fine, doesn't look like we are using the checksum anywhere anyway (at least yet).

efiop commented 2 years ago

Oh, looks like we haven't had PRs created from forks in awhile and the CI is failing due to abscent credentials 🙁 I guess this should be pretty safe to merge as is for now (with review addressed).

shcheklein commented 2 years ago

Oh, looks like we haven't had PRs created from forks in awhile and the CI is failing due to abscent credentials 🙁

Need to change the GH action (I'll do this). Going to use pull_request_target with the environments, not sure if it's the way that we use now in the DVC projects.

Thanks, @efiop !

efiop commented 2 years ago

Nice catch! Thank you, @simone-viozzi !