Skip to content
This repository has been archived by the owner on Dec 22, 2023. It is now read-only.

PMI-753: Read Google auth credentials from environment variables #12

Merged
merged 2 commits into from Nov 6, 2018

Conversation

robinmitra
Copy link
Contributor

@robinmitra robinmitra commented Nov 5, 2018

The implementation is quite similar to how we do this for Piwik token.

Since Pygsheets doesn't currently support reading credentials from an
in-memory data structure, we have had to implement creating a temporary
credentials file to do the job. We may revisit this later after the
next major release of Pygsheets.

Also note that, at present, dotenv package doesn't support multi-line
environment variables properly. Therefore, when running locally, the
environment variable GOOGLE_AUTH_PRIVATE_KEY cannot be defined in
.env file and must be defined in the shell. There is a PR currently
open on the Github repo of dotenv which fixes this behaviour, so we'll
revisit this later.

The implementation is quite similar to how we do this for Piwik token.
Since `Pygsheets` doesn't currently support reading credentials from an
in-memory data structure, we have had to implement creating a temporary
credentials file to do the job. We may revisit this later after the
next major release of `Pygsheets`.

Also note that, at present, `dotenv` package doesn't support multi-line
environment variables properly. Therefore, when running locally, the
environment variable `GOOGLE_AUTH_PRIVATE_KEY` cannot be defined in
`.env` file and must be defined in the shell. There is a PR currently
open on Github repo of `dotenv` which fixes this behaviour, so we'll
revisit this later.
performance/reports/rp.py Outdated Show resolved Hide resolved
performance/reports/rp.py Outdated Show resolved Hide resolved
galund
galund previously approved these changes Nov 6, 2018
Copy link
Contributor

@galund galund left a comment

Choose a reason for hiding this comment

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

Small additional comment suggested but apart from that LGTM :)

make it throw exception if a required environment variable doesn't exist.
Copy link
Contributor

@brenetic brenetic left a comment

Choose a reason for hiding this comment

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

🚀

@robinmitra robinmitra merged commit 9711827 into master Nov 6, 2018
@robinmitra robinmitra deleted the pmi-753-env-var-for-google-sheet branch November 6, 2018 17:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants