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

Private key filename #44

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Alex-Monahan
Copy link
Contributor

This allows you to refer directly to the JSON file that Google outputs instead of having to pass in a key as a string.

One consequence is that the key itself is not persisted within the secret, just the filename. I leave fixing that for another PR!

And, this time, I tested with a debug build so it should actually build and run on other people's machines... In theory!!

Closes #42.

@Alex-Monahan
Copy link
Contributor Author

Well, the windows errors look like they are coming from DuckDB's httplib that I added to the includes somehow. Odd. I'm not sure where to go next on that, but I can look next week.

@archiewood
Copy link
Member

i think this probably means you have the httplib on your machine but not in CI

@archiewood
Copy link
Member

might be a modification to CMakeLists but i'm not expert

@archiewood
Copy link
Member

Love the functionality in this PR

Some general thoughts:

  • I dont like having two HTTP libs - both openSSL and httplib kind of do the same thing

    • The perform_http_request function is currently where we are putting all queries through
    • I think httplib is probably the way to go long term, seems much easier, less ability to screw up
    • in the short term may be easier just to remove httplib and just use the perform_http_request function
  • It feels "wrong" to have to get a new token each request. I understand that it's because we are not handling token refresh

    • not averse to this in the short term
      • copypasta in read/copy feels suboptimal though, should abstract
    • long term feels like we should save the token somewhere (perhaps in the secrets manager token type we already have?) and then if the token expires, request a new one

-- (see "Getting a Google API Access Private Key" below)
CREATE SECRET (
TYPE gsheet,
PROVIDER private_key,
Copy link
Member

Choose a reason for hiding this comment

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

i think google would normally refer to this as key_file

@archiewood
Copy link
Member

archiewood commented Nov 20, 2024

Actually, it looks like we might be able to avoid OAuth all together if we are using a service account:

https://developers.google.com/identity/protocols/oauth2/service-account#jwt-auth

This seems like it might be much simpler.

  1. read the service account
  2. contsruct a JWT
  3. pass the JWT as a bearer token

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.

Add Authentication from keyfile.json
2 participants