Skip to content

pass apiClient to determineBaseRepo - #943

Merged
mislav merged 1 commit into
masterfrom
reauth-bug
May 18, 2020
Merged

mislav merged 1 commit into
masterfrom
reauth-bug

Conversation

@vilmibm

@vilmibm vilmibm commented May 15, 2020

Copy link
Copy Markdown
Contributor

Fixes #914

It is Mislav's and my hypothesis that the failed reauth issue was caused by having multiple
apiClient instances during a command's lifetime. The second apiClients are created inside of
determineBaseRepo; this PR changes determineBaseRepo to just accept a pre-existing apiClient.
It was only creating its own to begin with in an attempt to reduce the length of its function
signature and for no other reason.

Our code had an unspoken assumption that only one apiClient is created
during the course of a command. Violating this assumption is fine in
almost all cases, but not when we need to do a re-auth to add a new
oauth scope to a user's token.

There is likely a more elegant solution to the problem but until then
this changes determineBaseRepo to use an existing apiClient.
@vilmibm
vilmibm requested review from mislav and probablycorey May 15, 2020 20:34

@mislav mislav left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing this!

@mislav
mislav merged commit 4d11732 into master May 18, 2020
@mislav mislav mentioned this pull request May 20, 2020
@mislav
mislav deleted the reauth-bug branch June 25, 2020 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Updated config is not correctly picked up after re-auth

2 participants