google-ads-python
google-ads-python copied to clipboard
Bringing in service account string capability
Description:
This PR adds the ability to provide credentials via an environment variable for use cases where adding a .json to the filesystem in the runtime environment is not feasible for security reasons.
Testing:
Called get_credentials in oauth2.py by declaring a "json_key_string" (item) with personal credentials in config_data (dictionary)
The result was that we received as a valid credentials object.
References:
Would highly recommend seeing this as similar approach has been executed in big query api library!
BQ: https://github.com/anelendata/tap-bigquery/blob/master/tap_bigquery/sync_bigquery.py Implementation: https://gitlab.1password.io/data/data-issues/-/issues/3347#note_3160542
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
@dmosorast, @dsprayberry, @Vi6hal, @bryantgray would love your take on this PR, thanks!
This looks useful, but can you add unit tests for these changes? https://github.com/googleads/google-ads-python/blob/main/tests/oauth2_test.py
@srijan-chaudhuri I'd love to consider these changes, however I won't be able to merge this until the CLA check is passed, and unit tests are added. I'm closing this for now, but please re-open when ready.
@dmosorast, @dsprayberry, @Vi6hal, @bryantgray would love your take on this PR, thanks!
I agree with @BenRKarl on having tests for the changes. (Well, and CLA check, though that's less my domain) I assume that we've been tagged for Singer usage of this library. In that context, it seems like a good change, I see how it could be helpful to have this option to add flexibility to the Singer tap config to just take in the raw SA values. With this as proposed, the whole config (handled as a secret) can be used as input to the Singer job instead of having to manage multiple files.