Add AutoRemove to HostConfig #1050
Conversation
| @Override | ||
| @JsonIgnore | ||
| public Boolean getAutoRemove() { | ||
| return hostConfig.getAutoRemove(); |
There was a problem hiding this comment.
don't proxy calls to hostconfig. If you need this value get hostconfig object and get value from it
There was a problem hiding this comment.
I am confused because I just did the same as it is already done with other HostConfig properties. I wanted to follow the same architectural implementation. Please, take a look at pidMode,oomKillDisable, publishAllPorts and so many others properties of HostConfig.
Codecov Report
@@ Coverage Diff @@
## master #1050 +/- ##
==========================================
- Coverage 59.34% 57.11% -2.23%
==========================================
Files 441 441
Lines 8692 8694 +2
Branches 528 528
==========================================
- Hits 5158 4966 -192
- Misses 3246 3455 +209
+ Partials 288 273 -15
Continue to review full report at Codecov.
|
|
Review status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @KostyaSha) src/main/java/com/github/dockerjava/core/command/CreateContainerCmdImpl.java, line 202 at r1 (raw file): Previously, rnacher (Ricard Nàcher Roig) wrote…
Done. Comments from Reviewable |
|
Can you help me to understand travis-ci errors? I don't know how to fix them. thanks |
| @@ -529,6 +530,13 @@ public CreateContainerCmd withAttachStdout(Boolean attachStdout) { | |||
| return this; | |||
| } | |||
|
|
|||
| @Override | |||
| public CreateContainerCmd withAutoRemove(Boolean autoRemove) { | |||
There was a problem hiding this comment.
remove this chain, you are adding method for hostconfig, not for createcontainerCmdImpl
There was a problem hiding this comment.
I am sorry but I don't understand your request. I mean that right now, all other parameters of HostConfig can be set following a chain of setters in both CreateContainerCmd/Impl and in HostConfig. You can take a look at withCapDrop, withBinds, withDevices and so on. Do you want that autoRemove attribute would be different to all others? Why should this attribute be different? What do I misunderstand?
There was a problem hiding this comment.
because it's old code that is keeping for backward compatibility. It's nightmware to proxy(chain) all possible fields via one class, docker-java is tied to api, not to CLI. So just remove redundant chain for new code.
There was a problem hiding this comment.
OK. I understood it!
What do you think about adding a @deprecated annotation to all these "old" methods, to avoid coding in this unwanted way? Maybe @silvestre (#1052) and I would not have done the same!
There was a problem hiding this comment.
I thought i documented it somewhere. In future methods will be just removed.
|
and remove extra new lines 👍 |
I would like to be able to set this flag to a container creation. This is was added a long time ago to docker API 1.25+
It is related to issues: #792 and #979
I think that comments done in #336 are overdue now. I mean that version docker API 1.25 was not released at that time.
This change is