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

Types of fields in ContainerConfiguration #9

Open
mausch opened this issue May 25, 2015 · 2 comments
Open

Types of fields in ContainerConfiguration #9

mausch opened this issue May 25, 2015 · 2 comments

Comments

@mausch
Copy link
Contributor

mausch commented May 25, 2015

In https://github.com/almoehi/reactive-docker/blob/master/src/main/scala/com/kolor/docker/api/entities/ContainerConfiguration.scala shouldn't image be a String instead of an Option[String], since it's a required value?

Also I think volumes should be just a Map[String, DockerVolume], same thing for exposedPorts, env and probably others.

@almoehi
Copy link
Owner

almoehi commented May 27, 2015

I think wrapping everything in Option[X] happened unintentionally and probably is mostly related to json deserializers where readNullable returns an Option[X]. Of course this could be changed to to make volumes a Map.empty
Changing Option[X] for map-like properties would cause existing code to break and require some small refactorings for current users.
Any comments from someone else ?

Regarding the image property: This is true for docker containers. However, ContainerConfiguration is shared by ContainerInfo and DockerImageInfo where image(and lots of other properties) can be empty or missing. (at least this was the case during early development which used 0.x docker versions - maybe it changed in the meantime ?)

@mausch
Copy link
Contributor Author

mausch commented May 29, 2015

Not sure about ContainerInfo / DockerImageInfo (though I can't imagine any scenario where the image information wouldn't be present), but even so, if the image is required in one context and not required in another context, then there should be different types modeling each case.

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

No branches or pull requests

2 participants