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

Update GoogleDriveLoader to allow bring your own credentials #605

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

jeffbryner
Copy link
Contributor

issue: #168

PR Description

  • Where "package" is genai, vertexai, or community
  • Use "docs: ..." for purely docs changes, "templates: ..." for template changes, "infra: ..." for CI changes
  • Example: "community: add foobar LLM"
    Adds 'bring your own credentials' to the drive loader to allow more flexibility in deployments (cloud run service accounts, command line, jupyter notebooks, colab, etc).

PR Description and Relevant issues:

  • Allows for declaration of credentials created elsewhere (outside this module) to be used
  • Relevant issues [(Port randomness)](issue: GoogleDriveLoader creates random ports #168)
  • Any dependencies required for this change: None

Adds 'bring your own credentials' to the drive loader to allow more flexibility in deployments (cloud run service accounts, command line, jupyter notebooks, colab, etc).

Relevant issues

Fixes issue: #168

🐛 Bug Fix
🧹 Refactoring

Testing(optional)

Tested in jupyter notebook

@@ -276,6 +278,11 @@ def _load_credentials(self) -> Any:
if self.token_path.exists():
creds = Credentials.from_authorized_user_file(str(self.token_path), SCOPES)

if self.credentials:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nits: maybe we could move it a few lines up to avoid a not needed Credentials.from_authorized_user_file execution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thought was the other options wouldn't be called since they don't/wouldn't exist if you just pass in credentials? i.e. I'd set it on init to only use credentials and no other option:

drive_loader = GoogleDriveLoader(credentials=credentials, folder_id='someid', recursive=True)

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, but what if smbd sends the token_path also?
it's not crucial, the important thing is that linter is failing, please take a look

P.S. https://github.com/langchain-ai/langchain-google?tab=readme-ov-file#local-development-dependencies describes how to run a linter locally if you wish.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, looks like trailing spaces. Fixed.

@lkuligin lkuligin merged commit d6749c3 into langchain-ai:main Nov 27, 2024
15 checks passed
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