Skip to content

Remove unnecessary call to /info endpoint #19973

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 3, 2016
Merged

Remove unnecessary call to /info endpoint #19973

merged 1 commit into from
Feb 3, 2016

Conversation

icecrime
Copy link
Contributor

@icecrime icecrime commented Feb 3, 2016

Sorry for this: #19891 was missing its logout counterpart.

@icecrime icecrime added this to the 1.10.0 milestone Feb 3, 2016
@icecrime icecrime force-pushed the cross_platforms_logout branch from 6713ba6 to 8ee7ad2 Compare February 3, 2016 18:32
@tiborvass
Copy link
Contributor

LGTM

@LK4D4
Copy link
Contributor

LK4D4 commented Feb 3, 2016

LGTM

@aaronlehmann
Copy link
Contributor

LGTM

@aaronlehmann aaronlehmann mentioned this pull request Feb 3, 2016
Merged
@dmcgowan
Copy link
Member

dmcgowan commented Feb 3, 2016

I find it unnecessary to call cli.electAuthServer() when using a private registry since the value will be immediately overwritten. It would make more sense to only call this on an else block for if len(cmd.Args()) > 0

@LK4D4
Copy link
Contributor

LK4D4 commented Feb 3, 2016

I agree with @dmcgowan

@icecrime
Copy link
Contributor Author

icecrime commented Feb 3, 2016

Good point, updating. Thanks @dmcgowan.

@icecrime icecrime force-pushed the cross_platforms_logout branch from 8ee7ad2 to a749fdb Compare February 3, 2016 20:31
@icecrime
Copy link
Contributor Author

icecrime commented Feb 3, 2016

Updated, PTAL!

@dmcgowan
Copy link
Member

dmcgowan commented Feb 3, 2016

LGTM

@LK4D4
Copy link
Contributor

LK4D4 commented Feb 3, 2016

@icecrime need rebase

@icecrime icecrime force-pushed the cross_platforms_logout branch from a749fdb to 2b50c52 Compare February 3, 2016 21:07
@icecrime
Copy link
Contributor Author

icecrime commented Feb 3, 2016

Updated (@aaronlehmann's PR was merged first, and included the earlier version of this PR).

@aaronlehmann
Copy link
Contributor

Thanks @icecrime. LGTM.

@tiborvass
Copy link
Contributor

Sorry was my bad

Remove unnecessary call to /info
243d0d6 
Avoid using the `/info` endpoint in the `login` and `logout` workflows
when the Registry endpoint is overriden by the user through the command
line.

Signed-off-by: Arnaud Porterie <arnaud.porterie@docker.com>
@icecrime icecrime force-pushed the cross_platforms_logout branch from 2b50c52 to 243d0d6 Compare February 3, 2016 21:11
@icecrime icecrime changed the title Enable cross-platforms logout from Registry Remove unnecessary call to /info endpoint Feb 3, 2016
@calavera
Copy link
Contributor

calavera commented Feb 3, 2016

LGTM

tiborvass added a commit that referenced this pull request Feb 3, 2016
@tiborvass
Merge pull request #19973 from icecrime/cross_platforms_logout
7ecba9c 
Remove unnecessary call to `/info` endpoint
@tiborvass tiborvass merged commit 7ecba9c into moby:master Feb 3, 2016
@icecrime icecrime deleted the cross_platforms_logout branch February 27, 2016 01:48
@icecrime icecrime mentioned this pull request Mar 10, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.10.0
Development

Successfully merging this pull request may close these issues.
7 participants
@icecrime @tiborvass @LK4D4 @aaronlehmann @dmcgowan @calavera @GordonTheTurtle