openstack
/
deb-swift
Code Issues Proposed changes

Untangle ServersPerPortStrategy.bind_ports 

The "method" `bind_ports` only gets called once on startup - and *during
it's execution* it gets mutated to a list inside `_reload_bind_ports` -
afterwards all references are only to the attribute `bind_ports`.

It 100% works, but at first glance seems dubious.  Renaming the method
do_bind_ports seems more obvious.

Change-Id: Ic01621e478f1140644e26ba41715d93963a00945
This commit is contained in:
Clay Gerrard 2017-06-09 15:29:21 -07:00
parent 537f9a3f64
commit 14b07ca726
2 changed files with 18 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
swift/common/wsgi.py
View File

@ -480,7 +480,7 @@ class WorkersStrategy(object):
return 0.5 return 0.5
def bind_ports(self): def do_bind_ports(self):
""" """
Bind the one listen socket for this strategy and drop privileges Bind the one listen socket for this strategy and drop privileges
(since the parent process will never need to bind again). (since the parent process will never need to bind again).
@ -731,7 +731,7 @@ class ServersPerPortStrategy(object):
return self.ring_check_interval return self.ring_check_interval
def bind_ports(self): def do_bind_ports(self):
""" """
Bind one listen socket per unique local storage policy ring port. Then Bind one listen socket per unique local storage policy ring port. Then
do all the work of drop_privileges except the actual dropping of do all the work of drop_privileges except the actual dropping of
@ -906,7 +906,7 @@ def run_wsgi(conf_path, app_section, *args, **kwargs):
capture_stdio(logger) capture_stdio(logger)
# Start listening on bind_addr/port # Start listening on bind_addr/port
error_msg = strategy.bind_ports() error_msg = strategy.do_bind_ports()
if error_msg: if error_msg:
logger.error(error_msg) logger.error(error_msg)
print(error_msg) print(error_msg)

30
test/unit/common/test_wsgi.py
View File

@ -722,8 +722,8 @@ class TestWSGI(unittest.TestCase):
mock_run_server): mock_run_server):
# Make sure the right strategy gets used in a number of different # Make sure the right strategy gets used in a number of different
# config cases. # config cases.
mock_per_port().bind_ports.return_value = 'stop early' mock_per_port().do_bind_ports.return_value = 'stop early'
mock_workers().bind_ports.return_value = 'stop early' mock_workers().do_bind_ports.return_value = 'stop early'
logger = FakeLogger() logger = FakeLogger()
stub__initrp = [ stub__initrp = [
{'__file__': 'test', 'workers': 2}, # conf {'__file__': 'test', 'workers': 2}, # conf
@ -745,7 +745,7 @@ class TestWSGI(unittest.TestCase):
self.assertEqual([], mock_per_port.mock_calls) self.assertEqual([], mock_per_port.mock_calls)
self.assertEqual([ self.assertEqual([
mock.call(stub__initrp[0], logger), mock.call(stub__initrp[0], logger),
mock.call().bind_ports(), mock.call().do_bind_ports(),
], mock_workers.mock_calls) ], mock_workers.mock_calls)
stub__initrp[0]['servers_per_port'] = 3 stub__initrp[0]['servers_per_port'] = 3
@ -760,7 +760,7 @@ class TestWSGI(unittest.TestCase):
self.assertEqual([], mock_per_port.mock_calls) self.assertEqual([], mock_per_port.mock_calls)
self.assertEqual([ self.assertEqual([
mock.call(stub__initrp[0], logger), mock.call(stub__initrp[0], logger),
mock.call().bind_ports(), mock.call().do_bind_ports(),
], mock_workers.mock_calls) ], mock_workers.mock_calls)
mock_per_port.reset_mock() mock_per_port.reset_mock()
@ -772,7 +772,7 @@ class TestWSGI(unittest.TestCase):
], logger.get_lines_for_level('error')) ], logger.get_lines_for_level('error'))
self.assertEqual([ self.assertEqual([
mock.call(stub__initrp[0], logger, servers_per_port=3), mock.call(stub__initrp[0], logger, servers_per_port=3),
mock.call().bind_ports(), mock.call().do_bind_ports(),
], mock_per_port.mock_calls) ], mock_per_port.mock_calls)
self.assertEqual([], mock_workers.mock_calls) self.assertEqual([], mock_workers.mock_calls)
@ -913,7 +913,7 @@ class TestServersPerPortStrategy(unittest.TestCase):
self.assertEqual(15, self.strategy.loop_timeout()) self.assertEqual(15, self.strategy.loop_timeout())
def test_bind_ports(self): def test_bind_ports(self):
self.strategy.bind_ports() self.strategy.do_bind_ports()
self.assertEqual(set((6006, 6007)), self.strategy.bind_ports) self.assertEqual(set((6006, 6007)), self.strategy.bind_ports)
self.assertEqual([ self.assertEqual([
@ -940,7 +940,7 @@ class TestServersPerPortStrategy(unittest.TestCase):
def test_bind_ports_ignores_setsid_errors(self): def test_bind_ports_ignores_setsid_errors(self):
self.mock_setsid.side_effect = OSError() self.mock_setsid.side_effect = OSError()
self.strategy.bind_ports() self.strategy.do_bind_ports()
self.assertEqual(set((6006, 6007)), self.strategy.bind_ports) self.assertEqual(set((6006, 6007)), self.strategy.bind_ports)
self.assertEqual([ self.assertEqual([
@ -969,7 +969,7 @@ class TestServersPerPortStrategy(unittest.TestCase):
self.assertIsNone(self.strategy.no_fork_sock()) self.assertIsNone(self.strategy.no_fork_sock())
def test_new_worker_socks(self): def test_new_worker_socks(self):
self.strategy.bind_ports() self.strategy.do_bind_ports()
self.all_bind_ports_for_node.reset_mock() self.all_bind_ports_for_node.reset_mock()
pid = 88 pid = 88
@ -1104,7 +1104,7 @@ class TestServersPerPortStrategy(unittest.TestCase):
], self.mock_drop_privileges.mock_calls) ], self.mock_drop_privileges.mock_calls)
def test_shutdown_sockets(self): def test_shutdown_sockets(self):
self.strategy.bind_ports() self.strategy.do_bind_ports()
with mock.patch('swift.common.wsgi.greenio') as mock_greenio: with mock.patch('swift.common.wsgi.greenio') as mock_greenio:
self.strategy.shutdown_sockets() self.strategy.shutdown_sockets()
@ -1144,7 +1144,7 @@ class TestWorkersStrategy(unittest.TestCase):
self.assertEqual(0.5, self.strategy.loop_timeout()) self.assertEqual(0.5, self.strategy.loop_timeout())
def test_binding(self): def test_binding(self):
self.assertIsNone(self.strategy.bind_ports()) self.assertIsNone(self.strategy.do_bind_ports())
self.assertEqual('abc', self.strategy.sock) self.assertEqual('abc', self.strategy.sock)
self.assertEqual([ self.assertEqual([
@ -1159,20 +1159,20 @@ class TestWorkersStrategy(unittest.TestCase):
self.assertEqual( self.assertEqual(
'bind_port wasn\'t properly set in the config file. ' 'bind_port wasn\'t properly set in the config file. '
'It must be explicitly set to a valid port number.', 'It must be explicitly set to a valid port number.',
self.strategy.bind_ports()) self.strategy.do_bind_ports())
def test_no_fork_sock(self): def test_no_fork_sock(self):
self.strategy.bind_ports() self.strategy.do_bind_ports()
self.assertIsNone(self.strategy.no_fork_sock()) self.assertIsNone(self.strategy.no_fork_sock())
self.conf['workers'] = 0 self.conf['workers'] = 0
self.strategy = wsgi.WorkersStrategy(self.conf, self.logger) self.strategy = wsgi.WorkersStrategy(self.conf, self.logger)
self.strategy.bind_ports() self.strategy.do_bind_ports()
self.assertEqual('abc', self.strategy.no_fork_sock()) self.assertEqual('abc', self.strategy.no_fork_sock())
def test_new_worker_socks(self): def test_new_worker_socks(self):
self.strategy.bind_ports() self.strategy.do_bind_ports()
pid = 88 pid = 88
sock_count = 0 sock_count = 0
for s, i in self.strategy.new_worker_socks(): for s, i in self.strategy.new_worker_socks():
@ -1217,7 +1217,7 @@ class TestWorkersStrategy(unittest.TestCase):
def test_shutdown_sockets(self): def test_shutdown_sockets(self):
self.mock_get_socket.return_value = mock.MagicMock() self.mock_get_socket.return_value = mock.MagicMock()
self.strategy.bind_ports() self.strategy.do_bind_ports()
with mock.patch('swift.common.wsgi.greenio') as mock_greenio: with mock.patch('swift.common.wsgi.greenio') as mock_greenio:
self.strategy.shutdown_sockets() self.strategy.shutdown_sockets()
self.assertEqual([ self.assertEqual([