Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SDK/client] Fixes authentication refreshing mechanism #3981

Merged
merged 1 commit into from
Jun 16, 2020

Conversation

numerology
Copy link

Fixes #3630

Synced with @chensun , we only need this refresh for inverse proxy style authentication because gcloud auth print-access-token has a 1 hour TTL while in _get_auth_token() we're using refresh token, which does not have such limit.

@kubeflow-bot
Copy link

This change is Reviewable

@numerology
Copy link
Author

/assign @chensun
/assign @Bobgy
/assign @IronPan
/assign @Ark-kun

@Ark-kun
Copy link
Contributor

Ark-kun commented Jun 12, 2020

Thank you for fixing the issue.
/lgtm
BTW, can the same approach also be used to fix #2625 ?

@numerology
Copy link
Author

Thank you for fixing the issue.
/lgtm
BTW, can the same approach also be used to fix #2625 ?

Thanks! Actually I think #2625 is fixed by #2626 IIUC, where refresh token is introduced.

I am under the impression that refresh token should not have 1 hour TTL. @chensun can correct me if I am wrong.

However if a user provide an existing token to the client I think they'll need to take care of the refreshing themselves.

@numerology
Copy link
Author

@numerology
Copy link
Author

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: numerology

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit c04b9f0 into kubeflow:master Jun 16, 2020
@Bobgy Bobgy added the cherrypick-approved area OWNER approves to cherry pick this PR to current active release branch label Jun 17, 2020
RedbackThomson pushed a commit to RedbackThomson/pipelines that referenced this pull request Jun 17, 2020
@Bobgy Bobgy added the cherrypicked cherry picked to release branch `release-x.y` label Jul 2, 2020
Bobgy pushed a commit that referenced this pull request Jul 2, 2020
Jeffwan pushed a commit to Jeffwan/pipelines that referenced this pull request Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved cherrypick-approved area OWNER approves to cherry pick this PR to current active release branch cherrypicked cherry picked to release branch `release-x.y` lgtm size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SDK] kfp.Client().wait_for_run_completion() cannot handle long execution
7 participants