Adding detection of IP address with ifcfg==0.11b6

kolibri/utils/cli.py

@@ -235,9 +235,24 @@ def start(port=None, daemon=True):
 
     if not daemon:
         logger.info("Running 'kolibri start' in foreground...")
+
     else:
         logger.info("Running 'kolibri start' as daemon (system service)")
 
+    __, urls = server.get_urls(listen_port=port)
+    if not urls:
+        logger.error(
+            "Could not detect an IP address that Kolibri binds to, but try "
+            "opening up the following addresses:\n")
+        urls = [
+            "http://{}:{}".format(ip, port) for ip in ("localhost", "127.0.0.1")
+        ]
+    else:
+        logger.info("Kolibri running on:\n")
+    for addr in urls:
+        sys.stderr.write("\t{}\n".format(addr))
+    sys.stderr.write("\n")
+
     # Daemonize at this point, no more user output is needed
     if daemon:
 
@@ -254,7 +269,7 @@ def start(port=None, daemon=True):
     server.start(port=port)
 
 
-def stop(sys_exit=True):
+def stop():
     """
     Stops the server unless it isn't running
     """
@@ -288,9 +303,7 @@ def stop(sys_exit=True):
             stopped = True
 
     if stopped:
-        logger.info("Server stopped")
-        if sys_exit:
-            sys.exit(0)
+        sys.exit(0)
 
 
 def status():

kolibri/utils/server.py

@@ -3,6 +3,7 @@
 import os
 
 import cherrypy
+import ifcfg
 import requests
 from django.conf import settings
 from kolibri.content.utils import paths
@@ -259,15 +260,20 @@ def get_status():  # noqa: max-complexity=16
     # raise NotRunning(STATUS_UNKNOW)
 
 
-def get_urls():
+def get_urls(listen_port=None):
     """
-    This is a stub, see: https://github.com/learningequality/kolibri/issues/1595
+    :param listen_port: if set, will not try to determine the listen port from
+                        other running instances.
     """
     try:
-        __, __, port = get_status()
+        if listen_port:
+            port = listen_port
+        else:
+            __, __, port = get_status()
         urls = []
-        for addr in [LISTEN_ADDRESS]:
-            urls.append("http://{}:{}/".format(addr, port))
+        interfaces = ifcfg.interfaces()
+        for interface in filter(lambda i: i['inet'], interfaces.values()):
+            urls.append("http://{}:{}/".format(interface['inet'], port))
         return STATUS_RUNNING, urls
     except NotRunning as e:
         return e.status_code, []

kolibri/utils/tests/test_cli.py

@@ -6,14 +6,28 @@
 import copy
 import logging
 import os
-import unittest
 
 import pytest
 from kolibri.utils import cli
 
 logger = logging.getLogger(__name__)
 
 
+LOG_LOGGER = []
+
+
+def log_logger(logger_instance, LEVEL, msg, args, **kwargs):
+    """
+    Monkeypatching for logging.Logger._log to scoop up log messages if we wanna
+    test something specific was logged.
+    """
+    LOG_LOGGER.append(
+        (LEVEL, msg)
+    )
+    # Call the original function
+    logger_instance.__log(LEVEL, msg, args, **kwargs)
+
+
 @pytest.fixture
 def conf():
     from kolibri.utils import conf
@@ -94,64 +108,81 @@ def test_plugin_with_no_plugin_class(conf):
     assert installed_apps_before == conf.config["INSTALLED_APPS"]
 
 
-class TestKolibriCLI(unittest.TestCase):
-
-    def test_cli(self):
-        logger.debug("This is a unit test in the main Kolibri app space")
-        # Test the -h
-        with self.assertRaises(SystemExit):
-            cli.main("-h")
-        with self.assertRaises(SystemExit):
-            cli.main("--version")
-
-    def test_parsing(self):
-        test_patterns = (
-            (['start'], {'start': True}, []),
-            (['stop'], {'stop': True}, []),
-            (['shell'], {'shell': True}, []),
-            (['manage', 'shell'], {'manage': True, 'COMMAND': 'shell'}, []),
-            (['manage', 'help'], {'manage': True, 'COMMAND': 'help'}, []),
-            (['manage', 'blah'], {'manage': True, 'COMMAND': 'blah'}, []),
-            (
-                ['manage', 'blah', '--debug', '--', '--django-arg'],
-                {'manage': True, 'COMMAND': 'blah', '--debug': True},
-                ['--django-arg']
-            ),
-            (
-                ['manage', 'blah', '--django-arg'],
-                {'manage': True, 'COMMAND': 'blah'},
-                ['--django-arg']
-            ),
-        )
-
-        for p, docopt_expected, django_expected in test_patterns:
-            docopt, django = cli.parse_args(p)
-
-            for k, v in docopt_expected.items():
-                assert docopt[k] == v
-
-            assert django == django_expected
-
-    @pytest.mark.django_db
-    def test_kolibri_listen_port_env(self):
-        """
-        Starts and stops the server, mocking the actual server.start()
-        Checks that the correct fallback port is used from the environment.
-        """
-        test_port = 1234
-        # ENV VARS are always a string
-        os.environ['KOLIBRI_LISTEN_PORT'] = str(test_port)
-
-        def start_mock(port, *args, **kwargs):
-            assert port == test_port
-
-        from kolibri.utils import server
-
-        orig_start = server.start
-
-        try:
-            server.start = start_mock
-            cli.start(daemon=False)
-            cli.stop(sys_exit=False)
-        finally:
-            server.start = orig_start
+@pytest.mark.django_db
+def test_kolibri_listen_port_env(monkeypatch):
+    """
+    Starts and stops the server, mocking the actual server.start()
+    Checks that the correct fallback port is used from the environment.
+    """
+
+    from kolibri.utils import server
+
+    def start_mock(port, *args, **kwargs):
+        assert port == test_port
+
+    monkeypatch.setattr(logging.Logger, '__log', logging.Logger._log, raising=False)
+    monkeypatch.setattr(logging.Logger, '_log', log_logger)
+    monkeypatch.setattr(server, 'start', start_mock)
+
+    test_port = 1234
+    # ENV VARS are always a string
+    os.environ['KOLIBRI_LISTEN_PORT'] = str(test_port)
+
+    server.start = start_mock
+    cli.start(daemon=False)
+    with pytest.raises(SystemExit, code=0):
+        cli.stop()
+
+    # Stop the server AGAIN, asserting that we can call the stop command
+    # on an already stopped server and will be gracefully informed about
+    # it.
+    with pytest.raises(SystemExit, code=0):
+        cli.stop()
+    assert "Already stopped" in LOG_LOGGER[-1][1]
+
+    def status_starting_up():
+        raise server.NotRunning(server.STATUS_STARTING_UP)
+
+    # Ensure that if a server is reported to be 'starting up', it doesn't
+    # get killed while doing that.
+    monkeypatch.setattr(server, 'get_status', status_starting_up)
+    with pytest.raises(SystemExit, code=server.STATUS_STARTING_UP):
+        cli.stop()
+    assert "Not stopped" in LOG_LOGGER[-1][1]
+
+
+def test_cli_usage():
+    # Test the -h
+    with pytest.raises(SystemExit, code=0):
+        cli.main("-h")
+    with pytest.raises(SystemExit, code=0):
+        cli.main("--version")
+
+
+def test_cli_parsing():
+    test_patterns = (
+        (['start'], {'start': True}, []),
+        (['stop'], {'stop': True}, []),
+        (['shell'], {'shell': True}, []),
+        (['manage', 'shell'], {'manage': True, 'COMMAND': 'shell'}, []),
+        (['manage', 'help'], {'manage': True, 'COMMAND': 'help'}, []),
+        (['manage', 'blah'], {'manage': True, 'COMMAND': 'blah'}, []),
+        (
+            ['manage', 'blah', '--debug', '--', '--django-arg'],
+            {'manage': True, 'COMMAND': 'blah', '--debug': True},
+            ['--django-arg']
+        ),
+        (
+            ['manage', 'blah', '--django-arg'],
+            {'manage': True, 'COMMAND': 'blah'},
+            ['--django-arg']
+        ),
+    )
+
+    for p, docopt_expected, django_expected in test_patterns:
+        docopt, django = cli.parse_args(p)
+
+        for k, v in docopt_expected.items():
+            assert docopt[k] == v
+
+        assert django == django_expected

requirements/base.txt

@@ -22,3 +22,4 @@ requests-toolbelt==0.7.1
 clint==0.5.1
 tzlocal==1.4
 python-dateutil==2.6.0
+ifcfg==0.11b6