Move messaging into cli options

The messaging is not really part of compose settings. It is an
infrastructure part. As such, it should really be set up as part of
pungi invocation, not compose configuration.

The documentation is updated to reflect this. Some updates to the
documentation are done as well: listing messages about ISOs and minor
formatting updates.

The test_compose.sh script can now accept additional command line
options and pass them on to pungi-koji to simplify testing.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

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'),