Merge #69 Move messaging into cli options and simplify it

bin/pungi-koji

@@ -26,7 +26,6 @@ locale.setlocale(locale.LC_ALL, "C")
 
 
 COMPOSE = None
-NOTIFIER = None
 
 
 def main():
@@ -116,6 +115,10 @@ def main():
         action="store_true",
         help="output version information and exit",
     )
+    parser.add_option(
+        "--notification-script",
+        help="script for sending progress notification messages"
+    )
 
     opts, args = parser.parse_args()
 
@@ -162,6 +165,7 @@ def main():
             parser.error(str(ex))
 
     from pungi.compose import Compose
+    import pungi.notifier
 
     logger = logging.Logger("Pungi")
     kobo.log.add_stderr_logger(logger)
@@ -174,8 +178,19 @@ def main():
     else:
         compose_dir = opts.compose_dir
 
-    compose = Compose(conf, topdir=compose_dir, debug=opts.debug_mode, skip_phases=opts.skip_phase, just_phases=opts.just_phase,
+    notifier = pungi.notifier.PungiNotifier(opts.notification_script)
-                      old_composes=opts.old_composes, koji_event=opts.koji_event, supported=opts.supported, logger=logger)
+
+    compose = Compose(conf,
+                      topdir=compose_dir,
+                      debug=opts.debug_mode,
+                      skip_phases=opts.skip_phase,
+                      just_phases=opts.just_phase,
+                      old_composes=opts.old_composes,
+                      koji_event=opts.koji_event,
+                      supported=opts.supported,
+                      logger=logger,
+                      notifier=notifier)
+    notifier.compose = compose
     kobo.log.add_file_logger(logger, compose.paths.log.log_file("global", "pungi.log"))
     COMPOSE = compose
     try:
@@ -188,19 +203,10 @@ def main():
 def run_compose(compose):
     import pungi.phases
     import pungi.metadata
-    import pungi.notifier
 
     errors = []
 
-    # initializer notifier
-    compose.notifier = pungi.notifier.PungiNotifier(compose)
-    try:
-        compose.notifier.validate()
-    except ValueError as ex:
-        errors.extend(["NOTIFIER: %s" % m for m in ex.message.split('\n')])
-
     compose.write_status("STARTED")
-    compose.notifier.send('start')
     compose.log_info("Host: %s" % socket.gethostname())
     compose.log_info("User name: %s" % getpass.getuser())
     compose.log_info("Working directory: %s" % os.getcwd())
@@ -241,7 +247,6 @@ def run_compose(compose):
                 errors.append("%s: %s" % (phase.name.upper(), i))
     if errors:
         for i in errors:
-            compose.notifier.send('abort')
             compose.log_error(i)
             print(i)
         sys.exit(1)
@@ -331,7 +336,6 @@ def run_compose(compose):
 
     compose.log_info("Compose finished: %s" % compose.topdir)
     compose.write_status("FINISHED")
-    compose.notifier.send('finish')
 
 
 if __name__ == "__main__":
@@ -346,8 +350,6 @@ if __name__ == "__main__":
             COMPOSE.write_status("DOOMED")
             import kobo.tback
             open(tb_path, "w").write(kobo.tback.Traceback().get_traceback())
-            if COMPOSE.notifier:
-                COMPOSE.notifier.send('doomed')
         else:
             print("Exception: %s" % ex)
             raise

doc/configuration.rst

@@ -551,6 +551,7 @@ Translate Paths Settings
 Example config
 --------------
 ::
+
     translate_paths = [
         ("/mnt/a", "http://b/dir"),
     ]
@@ -558,42 +559,7 @@ Example config
 Example usage
 -------------
 ::
+
     >>> from pungi.paths import translate_paths
     >>> print translate_paths(compose_object_with_mapping, "/mnt/a/c/somefile")
     http://b/dir/c/somefile
-
-
-Progress notification
-=====================
-
-*Pungi* has the ability to emit notification messages about progress and
-status. These can be used to e.g. send messages to *fedmsg*. This is
-implemented by actually calling a separate script.
-
-The script will be called with one argument describing action that just
-happened. A JSON-encoded object will be passed to standard input to provide
-more information about the event. At least, the object will contain a
-``compose_id`` key.
-
-Currently these messages are sent:
-
- * ``start`` -- when composing starts
- * ``abort`` -- when compose is aborted due to incorrect configuration
- * ``finish`` -- on successful finish of compose
- * ``doomed`` -- when an error happens
- * ``phase-start`` -- on start of a phase
- * ``phase-stop`` -- when phase is finished
-
-For phase related messages ``phase_name`` key is provided as well.
-
-The script is invoked in compose directory and can read other information
-there.
-
-A ``pungi-fedmsg-notification`` script is provided and understands this
-interface.
-
-Config options
---------------
-
-**notification_script**
-    (*str*) -- executable to be invoked to send the message

doc/index.rst

@@ -15,6 +15,7 @@ Contents:
     contributing
     testing
     configuration
+    messaging
 
 
 Indices and tables

doc/messaging.rst

@@ -0,0 +1,39 @@
+.. _messaging:
+
+Progress notification
+=====================
+
+*Pungi* has the ability to emit notification messages about progress and
+general status of the compose. These can be used to e.g. send messages to
+*fedmsg*. This is implemented by actually calling a separate script.
+
+The script will be called with one argument describing action that just
+happened. A JSON-encoded object will be passed to standard input to provide
+more information about the event. At the very least, the object will contain a
+``compose_id`` key.
+
+Currently these messages are sent:
+
+ * ``status-change`` -- when composing starts, finishes or fails; a ``status``
+   key is provided to indicate details
+ * ``phase-start`` -- on start of a phase
+ * ``phase-stop`` -- when phase is finished
+ * ``createiso-targets`` -- with a list of images to be created
+ * ``createiso-imagedone`` -- when any single image is finished
+ * ``createiso-imagefail`` -- when any single image fails to create
+
+For phase related messages ``phase_name`` key is provided as well.
+
+The script is invoked in compose directory and can read other information
+there.
+
+A ``pungi-fedmsg-notification`` script is provided and understands this
+interface.
+
+Setting it up
+-------------
+
+The script should be provided as a command line argument
+``--notification-script``. ::
+
+    --notification_script=pungi-fedmsg-notification

pungi/compose.py

@@ -92,7 +92,7 @@ def get_compose_dir(topdir, conf, compose_type="production", compose_date=None,
 
 
 class Compose(kobo.log.LoggingBase):
-    def __init__(self, conf, topdir, debug=False, skip_phases=None, just_phases=None, old_composes=None, koji_event=None, supported=False, logger=None):
+    def __init__(self, conf, topdir, debug=False, skip_phases=None, just_phases=None, old_composes=None, koji_event=None, supported=False, logger=None, notifier=None):
         kobo.log.LoggingBase.__init__(self, logger)
         # TODO: check if minimal conf values are set
         self.conf = conf
@@ -102,7 +102,7 @@ class Compose(kobo.log.LoggingBase):
         self.just_phases = just_phases or []
         self.old_composes = old_composes or []
         self.koji_event = koji_event
-        self.notifier = None
+        self.notifier = notifier
 
         # intentionally upper-case (visible in the code)
         self.DEBUG = debug
@@ -241,6 +241,7 @@ class Compose(kobo.log.LoggingBase):
             self.log_error(msg)
             raise RuntimeError(msg)
         open(os.path.join(self.topdir, "STATUS"), "w").write(stat_msg + "\n")
+        self.notifier.send('status-change', status=stat_msg)
 
     def get_status(self):
         path = os.path.join(self.topdir, "STATUS")

pungi/notifier.py

@@ -17,7 +17,6 @@ import json
 import threading
 
 from kobo import shortcuts
-from pungi.checks import validate_options
 
 
 class PungiNotifier(object):
@@ -27,23 +26,10 @@ class PungiNotifier(object):
     script fails, a warning will be logged, but the compose process will not be
     interrupted.
     """
-    config_options = (
+    def __init__(self, cmd):
-        {
+        self.cmd = cmd
-            "name": "notification_script",
-            "expected_types": [str],
-            "optional": True
-        },
-    )
-
-    def __init__(self, compose):
-        self.compose = compose
         self.lock = threading.Lock()
 
-    def validate(self):
-        errors = validate_options(self.compose.conf, self.config_options)
-        if errors:
-            raise ValueError("\n".join(errors))
-
     def _update_args(self, data):
         """Add compose related information to the data."""
         data.setdefault('compose_id', self.compose.compose_id)
@@ -58,14 +44,13 @@ class PungiNotifier(object):
         Unless you specify it manually, a ``compose_id`` key with appropriate
         value will be automatically added.
         """
-        script = self.compose.conf.get('notification_script', None)
+        if not self.cmd:
-        if not script:
             return
 
         self._update_args(kwargs)
 
         with self.lock:
-            ret, _ = shortcuts.run((script, msg),
+            ret, _ = shortcuts.run((self.cmd, msg),
                                    stdin_data=json.dumps(kwargs),
                                    can_fail=True,
                                    workdir=self.compose.paths.compose.topdir(),

tests/test_compose.sh

@@ -9,4 +9,4 @@ pungi-koji \
 --target-dir=_composes \
 --old-composes=_composes \
 --config=data/dummy-pungi.conf \
---test
+--test "$@"

tests/test_notifier.py

@@ -13,21 +13,10 @@ from pungi.notifier import PungiNotifier
 
 
 class TestNotifier(unittest.TestCase):
-    def test_incorrect_config(self):
-        compose = mock.Mock(
-            conf={'notification_script': [1, 2]}
-        )
-
-        n = PungiNotifier(compose)
-        with self.assertRaises(ValueError) as err:
-            n.validate()
-            self.assertIn('notification_script', err.message)
-
     @mock.patch('kobo.shortcuts.run')
     def test_invokes_script(self, run):
         compose = mock.Mock(
             compose_id='COMPOSE_ID',
-            conf={'notification_script': 'run-notify'},
             paths=mock.Mock(
                 compose=mock.Mock(
                     topdir=mock.Mock(return_value='/a/b')
@@ -37,7 +26,8 @@ class TestNotifier(unittest.TestCase):
 
         run.return_value = (0, None)
 
-        n = PungiNotifier(compose)
+        n = PungiNotifier('run-notify')
+        n.compose = compose
         data = {'foo': 'bar', 'baz': 'quux'}
         n.send('cmd', **data)
 
@@ -48,9 +38,7 @@ class TestNotifier(unittest.TestCase):
 
     @mock.patch('kobo.shortcuts.run')
     def test_does_not_run_without_config(self, run):
-        compose = mock.Mock(conf={})
+        n = PungiNotifier(None)
-
-        n = PungiNotifier(compose)
         n.send('cmd', foo='bar', baz='quux')
         self.assertFalse(run.called)
 
@@ -59,7 +47,6 @@ class TestNotifier(unittest.TestCase):
         compose = mock.Mock(
             compose_id='COMPOSE_ID',
             log_warning=mock.Mock(),
-            conf={'notification_script': 'run-notify'},
             paths=mock.Mock(
                 compose=mock.Mock(
                     topdir=mock.Mock(return_value='/a/b')
@@ -69,7 +56,8 @@ class TestNotifier(unittest.TestCase):
 
         run.return_value = (1, None)
 
-        n = PungiNotifier(compose)
+        n = PungiNotifier('run-notify')
+        n.compose = compose
         n.send('cmd')
 
         run.assert_called_once_with(('run-notify', 'cmd'),