[PATCH 1 of 7] [pyramid/ctl] pyramid command will always run in foreground from now on

Laurent Peuch cortex at worlddomination.be
Sun Sep 1 04:06:57 CEST 2019


# HG changeset patch
# User Laurent Peuch <cortex at worlddomination.be>
# Date 1558480615 -7200
#      Wed May 22 01:16:55 2019 +0200
# Node ID a63c93f27994cbb440a0431c1809166216eaa712
# Parent  1a5eb9a9f5b42fe7dd5f5fc38f3007a884d0a49d
[pyramid/ctl] pyramid command will always run in foreground from now on

Daemonization used to make sens in the past, but today "pyramid" command is
only used for dev and should never be used for deployement (you should use a
wsgi server for that instead) so daemonization doesn't fit any use case
anymore.

Closes #17232923

diff --git a/cubicweb/pyramid/pyramidctl.py b/cubicweb/pyramid/pyramidctl.py
--- a/cubicweb/pyramid/pyramidctl.py
+++ b/cubicweb/pyramid/pyramidctl.py
@@ -25,8 +25,6 @@ The reloading strategy is heavily inspir
 the pyramid script 'pserve'.
 """
 
-import atexit
-import errno
 import os
 import signal
 import sys
@@ -35,7 +33,8 @@ import threading
 import subprocess
 import warnings
 
-from cubicweb import ExecutionError
+from logilab.common.configuration import merge_options
+
 from cubicweb.cwconfig import CubicWebConfiguration as cwcfg
 from cubicweb.cwctl import CWCTL, InstanceCommand, init_cmdline_log_threshold
 from cubicweb.pyramid import wsgi_application_from_cwconfig
@@ -97,18 +96,14 @@ class PyramidStartHandler(InstanceComman
     name = 'pyramid'
     actionverb = 'started'
 
-    options = (
-        ('no-daemon',
-         {'action': 'store_true',
-          'help': 'Run the server in the foreground.'}),
+    options = merge_options((
         ('debug-mode',
          {'action': 'store_true',
           'help': 'Activate the repository debug mode ('
-                  'logs in the console and the debug toolbar).'
-                  ' Implies --no-daemon'}),
+                  'logs in the console and the debug toolbar).'}),
         ('debug',
          {'short': 'D', 'action': 'store_true',
-          'help': 'Equals to "--debug-mode --no-daemon --reload"'}),
+          'help': 'Equals to "--debug-mode --reload"'}),
         ('reload',
          {'action': 'store_true',
           'help': 'Restart the server if any source file is changed'}),
@@ -148,7 +143,7 @@ class PyramidStartHandler(InstanceComman
           'metavar': 'key1:value1,key2:value2',
           'default': {},
           'help': 'override <key> configuration file option with <value>.'}),
-    )
+    ) + InstanceCommand.options)
 
     _reloader_environ_key = 'CW_RELOADER_SHOULD_RUN'
 
@@ -178,92 +173,6 @@ class PyramidStartHandler(InstanceComman
         arg = win32api.GetShortPathName(arg)
         return arg
 
-    def _remove_pid_file(self, written_pid, filename):
-        current_pid = os.getpid()
-        if written_pid != current_pid:
-            # A forked process must be exiting, not the process that
-            # wrote the PID file
-            return
-        if not os.path.exists(filename):
-            return
-        with open(filename) as f:
-            content = f.read().strip()
-        try:
-            pid_in_file = int(content)
-        except ValueError:
-            pass
-        else:
-            if pid_in_file != current_pid:
-                msg = "PID file %s contains %s, not expected PID %s"
-                self.out(msg % (filename, pid_in_file, current_pid))
-                return
-        self.info("Removing PID file %s" % filename)
-        try:
-            os.unlink(filename)
-            return
-        except OSError as e:
-            # Record, but don't give traceback
-            self.out("Cannot remove PID file: (%s)" % e)
-        # well, at least lets not leave the invalid PID around...
-        try:
-            with open(filename, 'w') as f:
-                f.write('')
-        except OSError as e:
-            self.out('Stale PID left in file: %s (%s)' % (filename, e))
-        else:
-            self.out('Stale PID removed')
-
-    def record_pid(self, pid_file):
-        pid = os.getpid()
-        self.debug('Writing PID %s to %s' % (pid, pid_file))
-        with open(pid_file, 'w') as f:
-            f.write(str(pid))
-        atexit.register(
-            self._remove_pid_file, pid, pid_file)
-
-    def daemonize(self, pid_file):
-        pid = live_pidfile(pid_file)
-        if pid:
-            raise ExecutionError(
-                "Daemon is already running (PID: %s from PID file %s)"
-                % (pid, pid_file))
-
-        self.debug('Entering daemon mode')
-        pid = os.fork()
-        if pid:
-            # The forked process also has a handle on resources, so we
-            # *don't* want proper termination of the process, we just
-            # want to exit quick (which os._exit() does)
-            os._exit(0)
-        # Make this the session leader
-        os.setsid()
-        # Fork again for good measure!
-        pid = os.fork()
-        if pid:
-            os._exit(0)
-
-        # @@: Should we set the umask and cwd now?
-
-        import resource  # Resource usage information.
-        maxfd = resource.getrlimit(resource.RLIMIT_NOFILE)[1]
-        if (maxfd == resource.RLIM_INFINITY):
-            maxfd = MAXFD
-        # Iterate through and close all file descriptors.
-        for fd in range(0, maxfd):
-            try:
-                os.close(fd)
-            except OSError:  # ERROR, fd wasn't open to begin with (ignored)
-                pass
-
-        if (hasattr(os, "devnull")):
-            REDIRECT_TO = os.devnull
-        else:
-            REDIRECT_TO = "/dev/null"
-        os.open(REDIRECT_TO, os.O_RDWR)  # standard input (0)
-        # Duplicate standard input to standard output and standard error.
-        os.dup2(0, 1)  # standard output (1)
-        os.dup2(0, 2)  # standard error (2)
-
     def restart_with_reloader(self, filelist_path):
         self.debug('Starting subprocess with file monitor')
 
@@ -337,11 +246,11 @@ class PyramidStartHandler(InstanceComman
     def pyramid_instance(self, appid):
         self._needreload = False
 
-        debugmode = self['debug-mode'] or self['debug']
         autoreload = self['reload'] or self['debug']
-        daemonize = not (self['no-daemon'] or debugmode or autoreload)
-
-        cwconfig = cwcfg.config_for(appid, debugmode=debugmode)
+
+        # debugmode=True is to force to have a StreamHandler used instead of
+        # writting the logs into a file in /tmp
+        cwconfig = cwcfg.config_for(appid, debugmode=True)
         filelist_path = os.path.join(cwconfig.apphome,
                                      '.pyramid-reload-files.list')
 
@@ -362,10 +271,6 @@ class PyramidStartHandler(InstanceComman
                 self['reload-interval'], extra_files,
                 filelist_path=filelist_path)
 
-        if daemonize:
-            self.daemonize(cwconfig['pid-file'])
-            self.record_pid(cwconfig['pid-file'])
-
         if self['dbglevel']:
             self['loglevel'] = 'debug'
             set_debug('|'.join('DBG_' + x.upper() for x in self['dbglevel']))
@@ -399,35 +304,6 @@ class PyramidStartHandler(InstanceComman
 CWCTL.register(PyramidStartHandler)
 
 
-def live_pidfile(pidfile):  # pragma: no cover
-    """(pidfile:str) -> int | None
-    Returns an int found in the named file, if there is one,
-    and if there is a running process with that process id.
-    Return None if no such process exists.
-    """
-    pid = read_pidfile(pidfile)
-    if pid:
-        try:
-            os.kill(int(pid), 0)
-            return pid
-        except OSError as e:
-            if e.errno == errno.EPERM:
-                return pid
-    return None
-
-
-def read_pidfile(filename):
-    if os.path.exists(filename):
-        try:
-            with open(filename) as f:
-                content = f.read()
-            return int(content.strip())
-        except (ValueError, IOError):
-            return None
-    else:
-        return None
-
-
 def _turn_sigterm_into_systemexit():
     """Attempts to turn a SIGTERM exception into a SystemExit exception."""
     try:
diff --git a/cubicweb/pyramid/test/test_ctl.py b/cubicweb/pyramid/test/test_ctl.py
new file mode 100644
--- /dev/null
+++ b/cubicweb/pyramid/test/test_ctl.py
@@ -0,0 +1,87 @@
+# copyright 2019 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# contact http://www.logilab.fr/ -- mailto:contact at logilab.fr
+#
+# This file is part of CubicWeb.
+#
+# CubicWeb is free software: you can redistribute it and/or modify it under the
+# terms of the GNU Lesser General Public License as published by the Free
+# Software Foundation, either version 2.1 of the License, or (at your option)
+# any later version.
+#
+# CubicWeb is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Lesser General Public License along
+# with CubicWeb.  If not, see <http://www.gnu.org/licenses/>.
+
+import os
+import logging
+import waitress
+import unittest
+from unittest.mock import patch, MagicMock
+
+from logilab.common.clcommands import CommandLine
+
+from cubicweb.cwconfig import CubicWebConfiguration as cwcfg
+from cubicweb.__pkginfo__ import version as cw_version
+
+from cubicweb.pyramid import pyramidctl
+from cubicweb.pyramid.pyramidctl import PyramidStartHandler
+
+
+class PyramidCommandTest(unittest.TestCase):
+    def setUp(self):
+        self.CWCTL = CommandLine('cubicweb-ctl', 'The CubicWeb swiss-knife.',
+                                 version=cw_version, check_duplicated_command=False)
+        cwcfg.load_cwctl_plugins()
+        self.CWCTL.register(pyramidctl.PyramidStartHandler)
+
+        fake_config = MagicMock()
+        fake_config.apphome = "/some/fake/path"
+
+        # pretend that this instance exists
+        config_patcher = patch.object(cwcfg, 'config_for', return_value=fake_config)
+        config_patcher.start()
+        self.addCleanup(config_patcher.stop)
+
+    @patch.object(os, "fork")
+    @patch.object(pyramidctl, "_generate_pyramid_ini_file")  # prevent io
+    @patch.object(pyramidctl, "wsgi_application_from_cwconfig")
+    @patch.object(waitress, "serve")
+    def test_run_in_foreground(self, *args):
+        with self.assertRaises(SystemExit) as cm:
+            self.CWCTL.run(["pyramid", "some_instance"])
+        self.assertEqual(cm.exception.code, 0)
+
+        os.fork.assert_not_called()
+
+    @patch.object(os, "fork")
+    @patch.object(pyramidctl, "_generate_pyramid_ini_file")  # prevent io
+    @patch.object(pyramidctl, "wsgi_application_from_cwconfig")
+    @patch.object(waitress, "serve")
+    def test_stream_handler_for_logging_not_in_debug(self, *args):
+        with self.assertRaises(SystemExit) as cm:
+            self.CWCTL.run(["pyramid", "some_instance"])
+        self.assertEqual(cm.exception.code, 0)
+
+        os.fork.assert_not_called()
+
+        self.assertEqual(len(logging.getLogger().handlers), 1)
+        isinstance(logging.getLogger().handlers[0], logging.StreamHandler)
+
+    @patch.object(os, "fork")
+    @patch.object(pyramidctl, "_generate_pyramid_ini_file")  # prevent io
+    @patch.object(pyramidctl, "wsgi_application_from_cwconfig")
+    @patch.object(PyramidStartHandler, "restart_with_reloader", return_value=0)
+    @patch.object(waitress, "serve")
+    def test_stream_handler_for_logging_in_debug(self, *args):
+        with self.assertRaises(SystemExit) as cm:
+            self.CWCTL.run(["pyramid", "some_instance", "-D"])
+        self.assertEqual(cm.exception.code, 0)
+
+        os.fork.assert_not_called()
+
+        self.assertEqual(len(logging.getLogger().handlers), 1)
+        isinstance(logging.getLogger().handlers[0], logging.StreamHandler)
diff --git a/doc/changes/3.27.rst b/doc/changes/3.27.rst
--- a/doc/changes/3.27.rst
+++ b/doc/changes/3.27.rst
@@ -46,6 +46,9 @@ Backwards incompatible changes
 * All "cubicweb-ctl" command only accept one instance argument from now one
   (instead of 0 to n)
 
+* 'pyramid' command will always run in the foreground now, by consequence the
+  option ``--no-daemon`` has been removed.
+
 * DBG_MS flag has been removed since it is not used anymore
 
 Deprecated code drops



More information about the cubicweb-devel mailing list