Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do an extra check to stop an unclean shutdown from breaking kolibri. #8883

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 18 additions & 8 deletions kolibri/utils/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def error_log(self, msg="", level=20, traceback=False):
return logger.log(level, msg)


def check_port_availability(host, port):
def port_is_available_on_host(host, port):
"""
Make sure the port is available for the server to start.
"""
Expand Down Expand Up @@ -152,7 +152,7 @@ def get_port(self, host):
if not self.values[p] and p not in self.occupied_ports
)
if port:
if check_port_availability(host, port):
if port_is_available_on_host(host, port):
self.values[port] = True
return port
except StopIteration:
Expand Down Expand Up @@ -603,11 +603,10 @@ def __init__(
if sys.platform == "darwin":
self.background = False

# Check if there are other kolibri instances running
# If there are, then we need to stop users from starting kolibri again.
pid, _, _, status = _read_pid_file(self.pid_file)

if status in IS_RUNNING and pid_exists(pid):
if (
self._kolibri_appears_to_be_running()
and self._kolibri_main_port_is_occupied()
):
logger.error(
"There is another Kolibri server running. "
"Please use `kolibri stop` and try again."
Expand Down Expand Up @@ -662,6 +661,17 @@ def __init__(
reload_plugin = ProcessControlPlugin(self)
reload_plugin.subscribe()

def _kolibri_appears_to_be_running(self):
# Check if there are other kolibri instances running
# If there are, then we need to stop users from starting kolibri again.
pid, _, _, status = _read_pid_file(self.pid_file)
return status in IS_RUNNING and pid_exists(pid)

def _kolibri_main_port_is_occupied(self):
if not self.serve_http:
return False
return not port_is_available_on_host(self.listen_address, self.port)

def _port_check(self, port):
# In case that something other than Kolibri occupies the port,
# check the port's availability.
Expand All @@ -672,7 +682,7 @@ def _port_check(self, port):
if (
not os.environ.get("LISTEN_PID", None)
and port
and not check_port_availability(self.listen_address, port)
and not port_is_available_on_host(self.listen_address, port)
):
# Port is occupied
logger.error(
Expand Down
8 changes: 6 additions & 2 deletions kolibri/utils/tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,15 @@ def test_unclean_shutdown(self, pid_exists_mock, read_pid_file_mock):
run_mock.assert_called()

@mock.patch("kolibri.utils.server.pid_exists")
def test_server_running(self, pid_exists_mock, read_pid_file_mock):
@mock.patch("kolibri.utils.server.wait_for_free_port")
def test_server_running(
self, wait_for_port_mock, pid_exists_mock, read_pid_file_mock
):
wait_for_port_mock.side_effect = OSError
pid_exists_mock.return_value = True
read_pid_file_mock.return_value = (1000, 8000, 8001, server.STATUS_RUNNING)
with self.assertRaises(SystemExit):
server.start()
server.start(port=8000)


class ServerSignalHandlerTestCase(TestCase):
Expand Down