CHE-5871: refactor DockerMachineStarter #5931
Conversation
garagatyi
added
the
target/branch
garagatyi
requested review from
evidolob,
riuvshin,
skabashnyuk and
vparfonov
as code owners
| */ | ||
| @Singleton | ||
| public class DockerApiHostEnvVariableProvisioner implements ConfigurationProvisioner { | ||
| private final Pair<String, String> result; |
There was a problem hiding this comment.
Looks like you don't need Pair here. You can save value of the following expression
format("tcp://%s:%s", dockerConnectorConfiguration.getDockerHost(), dockerDaemonUri.getPort())
and then just use DockerConnectorConfiguration.DOCKER_HOST_PROPERTY constant as name of env variable.
| if (isNullOrEmpty(extraHosts)) { | ||
| this.extraHosts = Collections.emptyList(); | ||
| } else { | ||
| this.extraHosts = Arrays.stream(extraHosts.split(",")) |
There was a problem hiding this comment.
i'd better inject an array of strings, so there is no need to do the split, AFAIK StringArrayConverter will do it for you.
There was a problem hiding this comment.
Looks like StringArrayConverter requires space between values. WDYT?
There was a problem hiding this comment.
It requires zero or many spaces, thus it should be fine
There was a problem hiding this comment.
My bad, you are right! I'll replace splitting.
|
Build # 3302 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/3302/ to view the results. |
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/3312/ |
Refactor DockerMachineStarter to clean it up and reduce its responsibilities. Move container configuration applying to InfrastructureProvisioners. Move classes to different packages to create some structure and reduce classes mess in a single package. Uncomment some commented tests. Signed-off-by: Oleksandr Garagatyi <ogaragat@redhat.com>
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/3315/ |
What does this PR do?
Refactor DockerMachineStarter to clean it up and reduce its responsibilities.
Move container configuration applying to InfrastructureProvisioners.
Move classes to different packages to create some structure and reduce classes mess in a single package.
Uncomment some commented tests.
What issues does this PR fix or reference?
Fixes #5871
Changelog
Refactor DockerMachineStarter to clean it up