CURATOR-114 - Modified the TestingServer to expose the restart() method#11
CURATOR-114 - Modified the TestingServer to expose the restart() method#11asfgit merged 3 commits intoapache:masterfrom
Conversation
on the underlying TestingZooKeeperServer. Modified all unit tests that were previously using the stop() and then recreate using existing temporary directory and port approach for restarting the server, so that they now just call the restart() method.
There was a problem hiding this comment.
Why only make this valid if the server had already been stopped? Just for consistency with the internal TestingZookeeperServer behaviour?
There was a problem hiding this comment.
Just following the current convention with the underlying TestingZookeeperServer. It throws an exception if restart() is called on an instance that is not in a STOPPED state. Are you suggesting that restart() should work from any state? Or just a LATENT or STOPPED state? Either way, it would require changes to the TestingZookeeperServer, but I don't think that's inherently a problem as the restart() method is only used by the TestingCluster, and I don't think modifying the behaviour would cause issues there.
There was a problem hiding this comment.
As a user, it would make sense for restart to work from any state. My mental model is akin to Unix services where restart is simply "stop ; start"
There was a problem hiding this comment.
I agree, I was surprised that restart was really just 'start if you've been stopped before'. It doesn't look like any test cases are using this code at the moment, so I'll modify the restart() to do a stop() if it's running, followed by a start().
server from any state other than CLOSED (because this server has had all of its state cleared).
There was a problem hiding this comment.
start argument is not actually used. Is it meant to be?
There was a problem hiding this comment.
Good point, yes it should be passed into the other constructor, will fix this up now.
The start flag is not currently used in any of the unit tests, but I think that it's useful to be able to create a server without starting it. There are cases where you want to start with a server that is not started.
next constructor.
4 participants
Modified the TestingServer to expose the restart() method on the underlying TestingZooKeeperServer. Modified all unit tests that were previously using the stop() and then recreate using existing temporary directory and port approach for restarting the server, so that they now just call the restart() method.