Skip to content
This repository has been archived by the owner on Mar 13, 2022. It is now read-only.

Fixing get Google default credentials method #21

Closed
wants to merge 3 commits into from

Conversation

gabrielgbim
Copy link

@gabrielgbim gabrielgbim commented Jul 20, 2017

This should fix the following error:

Traceback (most recent call last):
  File "watch-namespace.py", line 4, in <module>
    config.load_kube_config()
  File "egg/kubernetes/config/kube_config.py", line 318, in load_kube_config
  File "egg/kubernetes/config/kube_config.py", line 221, in load_and_set
  File "egg/kubernetes/config/kube_config.py", line 160, in _load_authentication
  File "egg/kubernetes/config/kube_config.py", line 176, in _load_gcp_token
  File "egg/kubernetes/config/kube_config.py", line 124, in <lambda>
NameError: global name 'GoogleCredentials' is not defined

@codecov-io
Copy link

Codecov Report

Merging #21 into master will decrease coverage by 0.5%.
The diff coverage is 55.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #21      +/-   ##
==========================================
- Coverage   94.69%   94.19%   -0.51%     
==========================================
  Files           9        9              
  Lines         698      706       +8     
==========================================
+ Hits          661      665       +4     
- Misses         37       41       +4
Impacted Files Coverage Δ
config/kube_config.py 94.53% <55.55%> (-2.04%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1110248...d1b1ea9. Read the comment docs.

@mbohlool
Copy link
Contributor

Yes, we need to fix this. I tried your fix and it almost worked, except it is taking a long time. The other fix is to run the auth-cmd directly (kubectl is doing that). I am investigating this more.

@mbohlool
Copy link
Contributor

I think we need to also write token back to kube config file. Can you please fix tests so we can merge this?

@gabrielgbim
Copy link
Author

gabrielgbim commented Jul 21, 2017

I'm trying to fix tests, but I couldn't find what is wrong.

Can you help me on how to run and identify what test has failed?

Running the following:

$:~/git/python-base$ export TOXENV=update-pep8
$:~/git/python-base$ ./run_tox.sh tox

Got:

  update-pep8: commands succeeded
  congratulations :)

@mbohlool
Copy link
Contributor

run scripts/update-pep8.sh

@mbohlool
Copy link
Contributor

mbohlool commented Jul 21, 2017

@gabrielgbim I sent a follow up PR for this to add expiration mechanism as well as persisting config. I will merge it after your PR. When you fixed tests here, can you review that PR?

@mbohlool
Copy link
Contributor

@gabrielgbim Are you going to fix this PR or I should go ahead with my other PR?

@gabrielgbim
Copy link
Author

@mbohlool You can go ahead with the other one.

@pokoli
Copy link

pokoli commented Jul 25, 2017

@gabrielgbim then you should probably close this one.

Also it will be gret if you can test and review the other PR.

Thanks for your time.

@mbohlool
Copy link
Contributor

Closing in favor of #22

@mbohlool mbohlool closed this Jul 25, 2017
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.

4 participants