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

Remove SslHttpClient? #215

Open
amihaiemil opened this issue Dec 13, 2018 · 11 comments
Open

Remove SslHttpClient? #215

amihaiemil opened this issue Dec 13, 2018 · 11 comments
Labels
bug Something isn't working postponed question Further information is requested

Comments

@amihaiemil
Copy link
Owner

amihaiemil commented Dec 13, 2018

I would like to remove this class and all the code (constructors in RemoteDocker, mostly) that makes any assumption regarding securing communication with a remote Docker instance.

My reasoning is that we shouldn't make any assumptions since we will most likely not cover all the possibilities and we are just polluting the library. If the user really needs it and knows what they are doing, they can provide a properly configured HttpClient when instantiating a RemoteDocker.

What do you think?

@amihaiemil
Copy link
Owner Author

@bkuzmic @llorllale @paulodamaso any thoughts here? :)

@bkuzmic
Copy link
Contributor

bkuzmic commented Dec 13, 2018

@amihaiemil I agree that you should remove it. As you said, there are just too many options.

@paulodamaso
Copy link
Contributor

paulodamaso commented Dec 13, 2018

@amihaiemil I agree, I like your idea of a properly configured HttpClient if the user need some different behavior (like ssl). I'll let #177 on hold while you decide this.

@amihaiemil amihaiemil added the bug Something isn't working label Dec 13, 2018
@0crat
Copy link
Collaborator

0crat commented Dec 13, 2018

Job #215 is now in scope, role is DEV

@0crat
Copy link
Collaborator

0crat commented Dec 13, 2018

Bug was reported, see §29: +15 point(s) just awarded to @amihaiemil/z

@llorllale
Copy link
Contributor

@amihaiemil what assumptions are you referring to?

@amihaiemil
Copy link
Owner Author

@llorllale well, maybe the fact that we assume everything to be in a single keystore?

And then, simply the fact that we ask for the passwords doesn't seem right: if the user really knows what they want to do (and understand how we are going to use that data), then it will be easy for them to configure their own HttpClient. Furthermore, who knows, maybe they want to pass through a Proxy. It just seems like a lot of possibilities and our configuration seems to be lousy. Doesn't it seem the same to you? :D

And besides, even if they want to use us, they still have to configure the keystore and truststore + passwords, before passing everything to us. If they are not able to configure their own HttpClient it means they do not understand the topic and won't be able to do it anyway.

@llorllale
Copy link
Contributor

@amihaiemil I understand your concerns. I do think the specific issues you've pointed out can be addressed by improving the design though.

I personally still think providing users some basic pre-configured clients is useful. See if this example makes any sense to you.

@amihaiemil amihaiemil added question Further information is requested postponed labels Dec 14, 2018
@amihaiemil
Copy link
Owner Author

@0crat out

@0crat
Copy link
Collaborator

0crat commented Dec 14, 2018

@0crat out (here)

@amihaiemil Job gh:amihaiemil/docker-java-api#215 is not assigned, can't get performer

@0crat
Copy link
Collaborator

0crat commented Dec 14, 2018

@0crat out (here)

@amihaiemil The job #215 is now out of scope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working postponed question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants