-
Notifications
You must be signed in to change notification settings - Fork 185
allow incluster to accept pass-in config #193
Conversation
/assign roycaihw |
Codecov Report
@@ Coverage Diff @@
## master #193 +/- ##
=========================================
Coverage ? 92.69%
=========================================
Files ? 13
Lines ? 1519
Branches ? 0
=========================================
Hits ? 1408
Misses ? 111
Partials ? 0
Continue to review full report at Codecov.
|
travis has passed: https://travis-ci.org/github/kubernetes-client/python-base/builds/686358803 |
config/incluster_config.py
Outdated
def __init__(self, | ||
token_filename, | ||
cert_filename, | ||
try_refresh_token, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change. Please add a default value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
config/incluster_config.py
Outdated
self._token_refresh_period = datetime.timedelta(minutes=1) | ||
|
||
def load_and_set(self, refresh_token=True): | ||
def load_and_set(self, client_configuration): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a separate method and keep the existing one backwards-compatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added default value
config/incluster_config.py
Outdated
def load_and_set(self, client_configuration=None): | ||
try_set_default = False | ||
if client_configuration is None: | ||
config = type.__call__(Configuration) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/config/client_configuration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: roycaihw, zshihang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
fixes the e2e tests in kubernetes-client/python/pull/1161.
in_cluster_config with refreshing is mutating the global Configuration class. since all the tests are running in a single process, e2e tests running after the unit tests will use a modified Configuration.