resolves issue #110 by adding connection mode #111
Conversation
…lready existing containers.
| if(map.containsKey(SHOULD_ALLOW_TO_CONNECT_TO_RUNNING_CONTAINERS)) { | ||
| cubeConfiguration.shouldAllowToConnectToRunningContainers = Boolean.parseBoolean(map.get(SHOULD_ALLOW_TO_CONNECT_TO_RUNNING_CONTAINERS)); | ||
| if(map.containsKey(CONNECTION_MODE)) { | ||
| cubeConfiguration.connectionMode = ConnectionMode.valueOf(ConnectionMode.class, map.get(CONNECTION_MODE)); |
There was a problem hiding this comment.
Add some better Exception handling then just the default IllegalArgumentException("No Enum constant " + type...) coming from valueOf
|
We're missing 'don't stop if leave' part don't we? As far as I can tell we always stop/destroy the Cube? |
|
Hi we are not leaving the don't stop if leave. To not stop a container we only need to set the container as prerunningcube. In case that you set connection mode to STARTORCONNECTXXX means that allowReconnect returns true, and then you can check if container is running. If it is the case then we send the prerunningcontainer event. In other cases (in all connection modes) we need to start the container, it doesn't matter if we are in STARTANDSTOP or in STARTORCONNECTXXX, because we can be sure that container is not running, so we start it. Then when the container is started only if connection mode is STARTORCONNECTANDLEAVE we must not stop the container. And what we do is fire the PreRunningEvent. From the point of view of STARTORCONNECTANDLEAVE it doesn't matter if container was started just now or two executions back, he only wants to notify the event to set to Cube that the container should not be stopped. |
|
mm, right. It's DockerCube that just don't call stop if it's in PRE_RUNNING state. The command events are still fired, but the Cube is in the wrong state.. |
|
So we should change this approach? PreRunningEvent change state of the cube. |
why this happens? |
|
AfterSuite is always called; https://github.com/lordofthejars/arquillian-cube/blob/master/docker/src/main/java/org/arquillian/cube/impl/client/CubeSuiteLifecycleController.java#L42 The Cube.stop/destroy methods are always called.. But the DockerCube is in the wrong state so it just returns instead of calling DockerExecutorClient. |
|
Well it will not happen anything simply that because it is in prerunning state it won't be stopped which is the desired behaviour. What I tried is to avoid having special cases, in the sense that a STARTORCONNECTANDLEAVE logic doesn't matter if container it has just started in current execution or not, it will leave the container without stopping it. |
|
I was more concerned with the Events being fired for extensions. But extensions should probably rely on BeforeStop/AfterStop and not StopCube. StopCube is always fired, but Before/AfterStop is just fired if it's actually stopped. So I think technically we're good here. Personally I found it a bit odd that the logic is a bit split; We handle PreConnect specially in the CubeSuiteLifecycleHandler but when it comes to stopping or not stopping it's handled in DockerCube just because it's placed in a non stoppable state. Wondering if PRE_RUNNING is not a State, but some kinda Mode. the State would be STARTED. You can't stop a Cube in PRE_RUNNING mode, but it's still started. hm.. |
|
Yes I implemented in this way for one reason and now I can't remember. I just wondered the same as you do, and finally I decided the move there. Probably to have all the same logic in the same place. Of course it would be better to have the start as well there but because we need to know if the container is running and so on and the logic is much complex I decided to move in upper level. Probably you are right that prerunning is a mode and not a state. |
|
@lordofthejars sure, make a 'improvment' issue on it |
resolves issue #110 by adding connection mode
resolves issue #110 by adding connection mode attribute for reusing already existing containers.