pungi

Clone

#1414 Port pungi's scripts to fedora-messaging
Merged 3 years ago by lsedlar. Opened 3 years ago by pingou.
pingou/pungi fedora-messaging  into  master

file modified
+26 -7 
@@ -1,26 +1,45 @@ 
 

  # -*- coding: utf-8 -*-
 

  

+ from __future__ import print_function
 

+ 

  import argparse
 

- import fedmsg
 

- import fedmsg.config
 

+ import fedora_messaging.api
 

+ import fedora_messaging.config
 

+ import fedora_messaging.exceptions
 

  import json
 

  import sys
 

  

  

  def send(cmd, data):
 

      topic = "compose.%s" % cmd.replace("-", ".").lower()
 

-     fedmsg.publish(topic=topic, modname="pungi", msg=data)
 

+     try:
 

+         msg = fedora_messaging.api.Message(topic="pungi.{}".format(topic), body=data)
 

+         fedora_messaging.api.publish(msg)
 

+     except fedora_messaging.exceptions.PublishReturned as e:
 

+         print("Fedora Messaging broker rejected message %s: %s" % (msg.id, e))
 

+         sys.exit(1)
 

+     except fedora_messaging.exceptions.ConnectionException as e:
 

+         print("Error sending message %s: %s" % (msg.id, e))
 

+         sys.exit(1)
 

+     except Exception as e:
 

+         print("Error sending fedora-messaging message: %s" % (e))
 

+         sys.exit(1)
 

  

  

  def main():
 

      parser = argparse.ArgumentParser()
 

      parser.add_argument("cmd")
 

+     parser.add_argument(
 

+         "--config",
 

+         dest="config",
 

+         help="fedora-messaging configuration file to use. "
 

+         "This allows overriding the default "
 

+         "/etc/fedora-messaging/config.toml.",
 

+     )
 

      opts = parser.parse_args()
 

  

-     config = fedmsg.config.load_config()
 

-     config["active"] = True  # Connect out to a fedmsg-relay instance
 

-     config["cert_prefix"] = "releng"  # Use this cert.
 

-     fedmsg.init(**config)
 

+     if opts.config:
 

+         fedora_messaging.config.conf.load_config(opts.config)
 

  

      data = json.load(sys.stdin)
 

      send(opts.cmd, data)

file modified
+31 -8 
@@ -15,12 +15,15 @@ 
 

  

  import argparse
 

  import datetime
 

- import fedmsg.config
 

  import json
 

  import os
 

  import sys
 

  import time
 

  

+ import fedora_messaging.api
 

+ import fedora_messaging.exceptions
 

+ 

+ 

  RESEND_INTERVAL = 300  # In seconds
 

  SLEEP_TIME = 5
 

  
@@ -40,15 +43,41 @@ 
 

      print("%s: %s" % (datetime.datetime.utcnow(), msg))
 

  

  

+ def send(cmd, data):
 

+     topic = "compose.%s" % cmd.replace("-", ".").lower()
 

+     try:
 

+         msg = fedora_messaging.api.Message(topic="pungi.{}".format(topic), body=data)
 

+         fedora_messaging.api.publish(msg)
 

+     except fedora_messaging.exceptions.PublishReturned as e:
 

+         print("Fedora Messaging broker rejected message %s: %s" % (msg.id, e))
 

+         sys.exit(1)
 

+     except fedora_messaging.exceptions.ConnectionException as e:
 

+         print("Error sending message %s: %s" % (msg.id, e))
 

+         sys.exit(1)
 

+     except Exception as e:
 

+         print("Error sending fedora-messaging message: %s" % e)
 

+         sys.exit(1)
 

+ 

+ 

  def main():
 

      parser = argparse.ArgumentParser()
 

      parser.add_argument("cmd")
 

+     parser.add_argument(
 

+         "--config",
 

+         dest="config",
 

+         help="fedora-messaging configuration file to use. "
 

+         "This allows overriding the default "
 

+         "/etc/fedora-messaging/config.toml.",
 

+     )
 

      opts = parser.parse_args()
 

  

      if opts.cmd != "ostree":
 

          # Not an announcement of new ostree commit, nothing to do.
 

          sys.exit()
 

  

+     if opts.config:
 

+         fedora_messaging.config.conf.load_config(opts.config)
 

+ 

      try:
 

          data = json.load(sys.stdin)
 

      except ValueError:
 
@@ -63,12 +92,6 @@ 
 

  

      path = "%s/objects/%s/%s.commitmeta" % (repo, commit[:2], commit[2:])
 

  

-     config = fedmsg.config.load_config()
 

-     config["active"] = True  # Connect out to a fedmsg-relay instance
 

-     config["cert_prefix"] = "releng"  # Use this cert.
 

-     fedmsg.init(**config)
 

-     topic = "compose.%s" % opts.cmd.replace("-", ".").lower()
 

- 

      def wait_for(msg, test, *args):
 

          time_slept = 0
 

          while not test(*args):
 
@@ -76,7 +99,7 @@ 
 

              time_slept += SLEEP_TIME
 

              if time_slept >= RESEND_INTERVAL:
 

                  ts_log("Repeating notification")
 

-                 fedmsg.publish(topic=topic, modname="pungi", msg=data)
 

+                 send(opts.cmd, data)
 

                  time_slept = 0
 

              time.sleep(SLEEP_TIME)

1 new commit added

  • Add python2-fedora-messaging as Requires in the spec file
3 years ago

3 new commits added

  • Add python2-fedora-messaging as Requires in the spec file
  • Port scripts/wait_for_signed_ostree_handler.py to fedora-messaging
  • Port scripts/fedmsg_notification.py to fedora-messaging
3 years ago

rebased onto 66f7c3454166fd8b57b5e018ecfe7ac7a2c6a072
3 years ago

why not also print the exception here? not doing so is only going to make debugging harder...

other than that, looks OK to me by eyeball anyway. bit hard to test locally.

Code looks good to me.

I would like to see the script exist with non-zero code if there was an error. Otherwise problems might go undetected for longer.

Apart from the error handling this looks good to me. The change in spec file is not needed, since the file is only used for testing RPM builds and has diverged from Fedora spec file already by quite a bit.

I'll adjust the PR in the early afternoon, I can't do it right now :)

rebased onto ad1a336
3 years ago

Turns out, I had the code open in a console :)

Looks OK to me. I honestly wonder if it's worth doing the error handling at all, if we're just going to exit 1 anyway, it would be simpler and probably more informative just to let exceptions happen; it's usually easier to debug a problem if you're looking at the full traceback than if something has "helpfully" intercepted and obfuscated it like this. But I wouldn't block the merge on it.

Looks OK to me. I honestly wonder if it's worth doing the error handling at all, if we're just going to exit 1 anyway, it would be simpler and probably more informative just to let exceptions happen; it's usually easier to debug a problem if you're looking at the full traceback than if something has "helpfully" intercepted and obfuscated it like this. But I wouldn't block the merge on it.

That makes sense to me, just let me know and I'm happy to let it crash :)

Should we also do the messages schema ? I might be better to take a bit more time and finish this completely instead of having to comeback to it later.

I really would prefer we do it now. The messages not being published breaks quite a lot of stuff. If the bridge was working and the fedmsgs were being published that would be one thing, but that is not the case.

Should we also do the messages schema ? I might be better to take a bit more time and finish this completely instead of having to comeback to it later.

This is currently impacting our ability to make rawhide compose so it's a little
time bit sensitive (and I have no experience yet w/ doing messages schemas and
how to handle them from a packaging point of view, so I'm likely not the
best/fastest/most efficient person to do this).

So... I was/am building a pungi package for f32-infra with this patch... so we can try that out and unblock ourselves, and this could go on to deal with the schema's or whatever else is needed before merging?

2 new commits added

  • Port scripts/wait_for_signed_ostree_handler.py to fedora-messaging
  • Port scripts/fedmsg_notification.py to fedora-messaging
3 years ago

Talking with @kevin I realized I had not added the --config argument in the second commit, so this is done

@kevin has deployed the package with the patch, it seems, and it's working. we're getting notifications in IRC, and the openQA scheduler picked them up too and they seem to be processing fine (no jobs have been scheduled yet as no composes with testable images have completed since the messages came back). thanks.

Let's merge this as is, and work on the schema later. I don't have any experience with that, and I expect it may be tricky since Pungi needs to support sending messages to different brokers, which it currently handles by passing JSON to a separate executable. This system might need to be redesigned to properly support schemas.

Pull-Request has been merged by lsedlar
3 years ago
Changes Summary 2
+26 -7
file changed
pungi/scripts/fedmsg_notification.py
+31 -8
file changed
pungi/scripts/wait_for_signed_ostree_handler.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.