Pass credentials directly to GCP connectors rather than through environment variable #1040
Conversation
austinweisgrau
force-pushed
the
gcs_auth
branch
2 times, most recently
from
1183f86
to
43a035a
Compare
|
Ok this should qualify as a true refactor, where no existing behavior is altered (except for a very hacky use of these classes), but the bug will go away. |
austinweisgrau
force-pushed
the
gcs_auth
branch
3 times, most recently
from
589237c
to
6394960
Compare
|
I haven't used the Admin connector - curious if someone else can test that. ... @KasiaHinkson have u used it? |
|
Just ensuring that it can authenticate is all thats needed |
austinweisgrau
force-pushed
the
gcs_auth
branch
from
6394960
to
01cb579
Compare
|
I have not used the Admin connector and this has been sitting in my inbox too long waiting for me to find the time to explore that and see if I could test it. I can go ahead and review it assuming that since this works for other connectors it will presumably work for the Admin connector, or I can keep waiting and try to make time next week? |
austinweisgrau
force-pushed
the
gcs_auth
branch
3 times, most recently
from
c825011
to
7f993b5
Compare
|
So, funny thing. I enabled the Google Admin API so I could test this out. Turns out, the changes in this PR did break the parsons GoogleAdmin connector, a small change in argument order on the new client request method. That is fixed here. Trouble is, this connector was actually already broken. I tested the connector on main and its methods return empty Tables every time. Digging in, every request returns an error, but the connector doesn't check for that and just ends up returning empty Tables. So the connector is actually still as broken as it was on main, new issue here #1058 to track that. This PR should be good to go though - the new authentication works as intended. |
austinweisgrau
force-pushed
the
gcs_auth
branch
2 times, most recently
from
72e2bcf
to
c142843
Compare
austinweisgrau
force-pushed
the
gcs_auth
branch
2 times, most recently
from
db660c5
to
7bd1d41
Compare
|
Sorry this has been waiting so long. @KasiaHinkson did you review this or does it still need a review? |
Avoids issue documented in move-coop#1039 where credentials for all GCP clients are stored in the same environment variable, leading to overwrites if multiple clients are initialized in the same environment.
Avoids mock credentials needing to match Google Service Account credential parsing
Pass credentials directly to GCP connectors rather than through shared environment variable. Avoids issue documented in #1039 where credentials for all GCP clients are stored in the same environment variable, leading to overwrites if multiple clients are initialized in the same environment.
TODO