Refactor stop_host method to use topology #292
Conversation
| 'workers': good_hosts} | ||
| self.upload_topology(topology) | ||
| self.cluster.stop_host(bad_host) | ||
| self.cluster.stop_host(self.cluster.slaves[0]) |
There was a problem hiding this comment.
you've already put the bad host name in the topology, so stopping the host should really do anything. Maybe we don't need stop_host at all, just upload bad worker/coordinator topologies all around. In any case it should be one or the other.
| # Change the topology to something that doesn't exist | ||
| ips = self.get_ip_address_dict() | ||
| down_hostname = self.get_down_hostname() | ||
| self.exec_cmd_on_host( |
There was a problem hiding this comment.
Why do we need all three of these sed commands? Doesn't the topology usually just contain the internal host name?
petroav
force-pushed
the
fix-intermittent
branch
from
d5ada45
to
0131baf
Compare
|
@rschlussel updated version that addresses your comments. I also added a commit that fixed the |
|
travis failures are real. It looks like you missed a few usages of stop_host |
|
looks good % fixing the test failures. |
|
Can you also revert the timeout increase (and make sure that this fixes the issue)? |
- The requests library handles connection resets from the server better than urllib.
- Stopping a host is now done implicitly by uploading a topology that points to a host that doesn't exist. This was done because stopping a container caused nasty race conditions.
petroav
force-pushed
the
fix-intermittent
branch
from
46cd187
to
0ab3512
Compare
|
@petroav what's the status of this? |
|
@rschlussel I put this on pause since we were seeing all those |
upload_topologyto mimic behavior ofstop_host.