-
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
fs.gdrive: Fix GDRIVE_CREDENTIALS_DATA for service accounts #7213
Conversation
Related: iterative/dvc-gdrive#20 |
dvc/fs/gdrive.py
Outdated
"You can also provide these using the GDRIVE_CREDENTIALS_DATA" | ||
"environment variable.\n" |
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.
I think we can just leave this message out.
@RCdeWit Thanks for the contribution! Would you be able to respond to update the PR to address the comments? |
Thanks for the review, folks. Incorporated the feedback! |
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.
Not saying it should block the P.R. but we should probably test for this (maybe in https://github.com/iterative/dvc/blob/30d6839a5f471fb02a7a2dcbd6a6616b71277762/tests/unit/remote/test_gdrive.py)
I'm working on part 2 of a blog post that covers this use case. I was hoping to release it somewhere over the next two weeks; any chance this fix will be merged by then? If not, my blog post will have to include the workaround.
|
`_validate_config()` had a bug: it would not take into account that Google Drive credentials can also be passed using `GDRIVE_CREDENTIALS_DATA` as documented here [1]. This patches the validation logic to also accept this environment variable as a means to provide service account credentials. Before, commands like `dvc push` would exit with: ``` ERROR: failed to push data to the cloud - To use service account, set `gdrive_service_account_json_file_path`, and optionally`gdrive_service_account_user_email` in DVC config Learn more at <https://man.dvc.org/remote/modify> ``` [1]: https://dvc.org/doc/user-guide/setup-google-drive-remote#authorization Co-authored-by: Laurens Duijvesteijn <[email protected]>
_validate_config()
had a bug: it would not take into account that GoogleDrive credentials can also be passed using
GDRIVE_CREDENTIALS_DATA
asdocumented here 1.
This patches the validation logic to also accept this environment variable as a
means to provide service account credentials.
Before, commands like
dvc push
would exit with:Thanks to @duijf for helping me diagnose.
Co-authored-by: Laurens Duijvesteijn [email protected]