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

Support same URL schemes as Docker #259

Merged
merged 3 commits into from
Jul 9, 2014
Merged

Support same URL schemes as Docker #259

merged 3 commits into from
Jul 9, 2014

Conversation

shin-
Copy link
Contributor

@shin- shin- commented Jul 3, 2014

Fixes #166

@bfirsh @mpetazzoni I think I'm accounting for all the current and future use cases with this, but please review and let me know if I missed something.

@shin-
Copy link
Contributor Author

shin- commented Jul 3, 2014

This is still a breaking change, the original implementation doesn't allow to leave the port unspecified while docker-py currently does.

We can either stick with it and release a new major (it's kinda overdue anyway), or allow the port to be unspecified, with the potential security implications that this has. LMK what you think.

@bfirsh
Copy link
Contributor

bfirsh commented Jul 3, 2014

LGTM

@bfirsh
Copy link
Contributor

bfirsh commented Jul 3, 2014

/cc @aanand

@aanand
Copy link
Contributor

aanand commented Jul 3, 2014

LGTM too. If the Docker CLI doesn't allow an unspecified port, I don't think docker-py should either - and if it's a breaking change, so be it!

@shin-
Copy link
Contributor Author

shin- commented Jul 9, 2014

Thanks for the review guys :) merging!

shin- added a commit that referenced this pull request Jul 9, 2014
@shin- shin- merged commit ed2b458 into master Jul 9, 2014
@shin- shin- deleted the 166-complete-parse-host branch August 22, 2014 15:04
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.

docker-py should support same URL schemes as Docker
3 participants