Skip to content

fs: simplify auth#211

Merged
efiop merged 3 commits into
masterfrom
auth
Jul 21, 2022
Merged

fs: simplify auth#211
efiop merged 3 commits into
masterfrom
auth

Conversation

@efiop

@efiop efiop commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

Compared to how we were using it in dvc before:

  1. doesn't require/accept tmp_dir at all, caches creds in
    appdirs("pydrive2fs", appauthor=False).
  2. caching is per client_id, so no risk of using mismatched client_id and
    cached creds. If user supplies a profile(defaults to default) - we'll
    also cache it by profile to support multiple google users using the same
    client_id.
  3. renamed service_account_json_file_path into client_json_file_path for
    consistency.
  4. renamed service_account_user_email into client_user_email for
    consistency.

Other than that, this is mostly old code.

Fixes #193

@efiop
efiop temporarily deployed to internal July 19, 2022 18:46 Inactive
@efiop
efiop temporarily deployed to internal July 19, 2022 18:47 Inactive
@efiop efiop changed the title fs: simplify auth [WIP] fs: simplify auth Jul 19, 2022
@efiop
efiop temporarily deployed to internal July 19, 2022 18:48 Inactive
Comment thread pydrive2/fs/spec.py Outdated
@efiop
efiop temporarily deployed to internal July 19, 2022 18:54 Inactive
@efiop
efiop temporarily deployed to internal July 19, 2022 19:23 Inactive
@efiop
efiop temporarily deployed to internal July 19, 2022 19:25 Inactive
@efiop
efiop temporarily deployed to internal July 19, 2022 19:29 Inactive
@efiop
efiop temporarily deployed to internal July 19, 2022 19:32 Inactive
@efiop
efiop temporarily deployed to internal July 19, 2022 19:40 Inactive
@efiop
efiop temporarily deployed to internal July 19, 2022 19:46 Inactive
@efiop
efiop temporarily deployed to internal July 19, 2022 19:56 Inactive
Comment thread pydrive2/fs/spec.py Outdated
Comment thread pydrive2/fs/spec.py Outdated
@efiop
efiop temporarily deployed to internal July 19, 2022 20:02 Inactive
Comment thread pydrive2/fs/spec.py Outdated
Comment thread pydrive2/fs/spec.py Outdated
Comment thread pydrive2/fs/spec.py
@efiop
efiop temporarily deployed to internal July 20, 2022 20:21 Inactive
@efiop
efiop temporarily deployed to internal July 20, 2022 20:23 Inactive
Comment thread docs/fsspec.rst Outdated
@@ -0,0 +1,93 @@
Fsspec Filesystem

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For now went with a recipe-based approach instead of documenting every option one-by-one

@efiop
efiop temporarily deployed to internal July 20, 2022 20:28 Inactive
@efiop
efiop temporarily deployed to internal July 20, 2022 20:47 Inactive
@efiop
efiop temporarily deployed to internal July 20, 2022 20:49 Inactive
@efiop
efiop temporarily deployed to internal July 20, 2022 21:04 Inactive
@efiop
efiop temporarily deployed to internal July 20, 2022 21:17 Inactive
@efiop

efiop commented Jul 20, 2022

Copy link
Copy Markdown
Contributor Author

For the record: can't add fs to the https://docs.iterative.ai/PyDrive2/pydrive2/ as sphinx doesn't like existing code. Will get back to it in a followup.

@efiop efiop changed the title [WIP] fs: simplify auth fs: simplify auth Jul 20, 2022
@efiop efiop changed the title fs: simplify auth [WIP] fs: simplify auth Jul 20, 2022
Comment thread docs/fsspec.rst
@efiop
efiop temporarily deployed to internal July 20, 2022 21:46 Inactive
@efiop
efiop temporarily deployed to internal July 20, 2022 21:47 Inactive
Comment thread pydrive2/fs/spec.py Outdated
@efiop

efiop commented Jul 20, 2022

Copy link
Copy Markdown
Contributor Author

@shcheklein Should be alright for now. Please take a look when you'll have time.

efiop added 3 commits July 21, 2022 01:31
Compared to how we were using it in dvc before:

1) doesn't require/accept `tmp_dir` at all, caches creds in
   `appdirs("pydrive2fs", appauthor=False)`.
2) caching is per `client_id`, so no risk of using mismatched client_id and
   cached creds. If user supplies a `profile`(defaults to `default`) - we'll
   also cache it by `profile` to support multiple google users using the same
   `client_id`.
3) renamed `service_account_json_file_path` into `client_json_file_path` for
   consistency.
4) renamed `service_account_user_email` into `client_user_email` for
   consistency.

Other than that, this is mostly old code.

Fixes #193
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.

fs: simplify auth

2 participants