PR#42: Connect to UMB and parse brew.sign.rpm - freshmaker

freshmaker

#42 Connect to UMB and parse brew.sign.rpm

Merged 8 years ago by jkaluza. Opened 8 years ago by cqi.

cqi/freshmaker connect-to-umb into master

Connect to UMB and parse brew.sign.rpm

Chenxiong Qi • 8 years ago

4a7c8df

conf/config.py

file modified

+12 -1

		`@@ -1,3 +1,5 @@`
		`+ # -- coding: utf-8 --`
		`+`
		`from os import path`


		`@@ -26,7 +28,6 @@`

		`SYSTEM = 'koji'`
		`MESSAGING = 'fedmsg' # or amq`
		`- MESSAGING_TOPIC_PREFIX = ['org.fedoraproject.prod']`
		`PDC_URL = 'http://modularity.fedorainfracloud.org:8080/rest_api/v1'`
		`PDC_INSECURE = True`
		`PDC_DEVELOP = True`
		`@@ -40,6 +41,16 @@`
		`# Available log levels are: debug, info, warn, error.`
		`LOG_LEVEL = 'info'`

		`+ MESSAGING_TOPIC_PREFIX = ['org.fedoraproject.prod']`
		`+`
		`+ # Parsers defined for parse specific messages`
		`+ PARSERS = [`
		`+ 'freshmaker.parsers.bodhi:BodhiUpdateCompleteStableParser',`
		`+ 'freshmaker.parsers.git:GitReceiveParser',`
		`+ 'freshmaker.parsers.koji:KojiTaskStateChangeParser',`
		`+ 'freshmaker.parsers.mbs:MBSModuleStateChangeParser',`
		`+ ]`
		`+`
		`# List of enabled composing handlers.`
		`HANDLERS = [`
		`"freshmaker.handlers.bodhi:BodhiUpdateCompleteStableHandler",`

conf/configrh.py

file added

+64

		`@@ -0,0 +1,64 @@`
		`+ # -- coding: utf-8 --`
		`+`
		`+ import os`
		`+`
		`+ from conf import config`
		`+`
		`+`
		`+ class BaseConfiguration(config.BaseConfiguration):`
		`+ MESSAGING_TOPIC_PREFIX = [`
		`+ # This is the queue name to receive messages from UMB.`
		`+ # Generally, it has format Consumer.client-[name].*.VirtualTopic.>`
		`+ #`
		`+ # - name is deteremined by the certificate requested. For example, a`
		`+ # certificate is requested with client name msg-client-bob, then in`
		`+ # queue name, it name should be bob, that is Consumer.client-bob.`
		`+ #`
		`+ # - * is any word you choose, which should be descriptive to this queue`
		`+ #`
		`+ # - > represents the hierarchy of topic name, e.g. eng.brew.sign.rpm`
		`+ os.environ['FRESHMAKER_MESSAGING_TOPIC_PREFIX'],`
		`+ ]`
		`+`
		`+ PARSERS = [`
		`+ 'freshmaker.parsers.brew.sign_rpm:BrewSignRpmParser',`
		`+ ]`
		`+`
		`+ HANDLERS = [`
		`+ ]`
		`+`
		`+`
		`+ class DevConfiguration(BaseConfiguration):`
		`+ DEBUG = True`
		`+ LOG_BACKEND = 'console'`
		`+ LOG_LEVEL = 'debug'`
		`+`
		`+ # Global network-related values, in seconds`
		`+ NET_TIMEOUT = 5`
		`+ NET_RETRY_INTERVAL = 1`
		`+`
		`+ KOJI_CONTAINER_SCRATCH_BUILD = True`
		`+`
		`+ LIGHTBLUE_VERIFY_SSL = False`
		`+`
		`+`
		`+ class TestConfiguration(BaseConfiguration):`
		`+ LOG_BACKEND = 'console'`
		`+ LOG_LEVEL = 'debug'`
		`+ DEBUG = True`
		`+`
		`+ SQLALCHEMY_DATABASE_URI = 'sqlite:///{0}'.format(`
		`+ os.path.join(config.dbdir, 'tests', 'test_freshmaker.db'))`
		`+`
		`+ MESSAGING = 'in_memory'`
		`+ PDC_URL = 'http://pdc.fedoraproject.org/rest_api/v1'`
		`+`
		`+ # Global network-related values, in seconds`
		`+ NET_TIMEOUT = 3`
		`+ NET_RETRY_INTERVAL = 1`
		`+ MBS_AUTH_TOKEN = "testingtoken"`
		`+`
		`+ KOJI_CONTAINER_SCRATCH_BUILD = True`
		`+`
		`+ LIGHTBLUE_SERVER_URL = '' # replace with real dev server url`
		`+ LIGHTBLUE_VERIFY_SSL = False`

fedmsg.d/freshmaker-rh.py.disabled

file added

+12

		`@@ -0,0 +1,12 @@`
		`+ # -- coding: utf-8 --`
		`+`
		`+ import os`
		`+`
		`+ config = {`
		`+ 'zmq_enabled': False,`
		`+ 'stomp_heartbeat': 1000,`
		`+ 'stomp_uri': os.environ['FRESHMAKER_STOMP_URI'],`
		`+ 'stomp_ssl_crt': os.environ['FRESHMAKER_STOMP_SSL_CRT'],`
		`+ 'stomp_ssl_key': os.environ['FRESHMAKER_STOMP_SSL_KEY'],`
		`+ 'validate_signatures': False,`
		`+ }`

fedmsg.d/freshmaker.py

file modified

		`@@ -1,3 +1,5 @@`
		`+ # -- coding: utf-8 --`
		`+`
		`import os`

		`config = {`

freshmaker/config.py

file modified

+15 -4

		`@@ -50,9 +50,11 @@`
		`# try getting config_file from os.environ`
		`if 'FRESHMAKER_CONFIG_FILE' in os.environ:`
		`config_file = os.environ['FRESHMAKER_CONFIG_FILE']`
		`+`
		`# try getting config_section from os.environ`
		`if 'FRESHMAKER_CONFIG_SECTION' in os.environ:`
		`config_section = os.environ['FRESHMAKER_CONFIG_SECTION']`
		`+`
		`# TestConfiguration shall only be used for running tests, otherwise...`
		`if any(['nosetests' in arg or 'noserunner.py' in arg or 'py.test' in arg or 'pytest.py' in arg for arg in sys.argv]):`
		`config_section = 'TestConfiguration'`
		`@@ -64,17 +66,22 @@`
		`# package -> /conf/config.py.`

		`elif ('FRESHMAKER_DEVELOPER_ENV' in os.environ and`
		`- os.environ['FRESHMAKER_DEVELOPER_ENV'].lower() in (`
		`- '1', 'on', 'true', 'y', 'yes')):`
		`+ os.environ['FRESHMAKER_DEVELOPER_ENV'].lower() in ('1', 'on', 'true', 'y', 'yes')):`
		`config_section = 'DevConfiguration'`
		`- from conf import config`
		`- config_module = config`
		`+ if 'FRESHMAKER_CONFIG_FILE' in os.environ:`
		`+ config_file = os.environ['FRESHMAKER_CONFIG_FILE']`
		`+ config_module = None`
		`+ else:`
		`+ from conf import config`
		`+ config_module = config`
		`+`
		`# try loading configuration from file`
		`if not config_module:`
		`try:`
		`config_module = imp.load_source('freshmaker_runtime_config',`
		`config_file)`
		`except:`
		`+ raise`
		`raise SystemError("Configuration file {} was not found."`
		`.format(config_file))`

		`@@ -132,6 +139,10 @@`
		`'type': int,`
		`'default': 30,`
		`'desc': 'Global network retry interval for read/write operations, in seconds.'},`
		`+ 'parsers': {`
		`+ 'type': list,`
		`+ 'default': [],`
		`+ 'desc': 'Parsers defined for parse specific messages.'},`
		`'handlers': {`
		`'type': list,`
		`'default': ["freshmaker.handlers.mbs:MBSModuleStateChangeHandler"],`

freshmaker/consumer.py

file modified

+12 -18

		`@@ -27,13 +27,9 @@`

		`import fedmsg.consumers`
		`import moksha.hub`
		`- import freshmaker.handlers`
		`- import freshmaker.parsers.mbs`
		`- import freshmaker.parsers.git`
		`- import freshmaker.parsers.bodhi`
		`- import freshmaker.parsers.koji`

		`from freshmaker import log, conf, messaging, events`
		`+ from freshmaker.utils import load_classes`


		`class FreshmakerConsumer(fedmsg.consumers.FedmsgConsumer):`
		`@@ -45,7 +41,6 @@`

		`def __init__(self, hub):`
		`# set topic before super, otherwise topic will not be subscribed`
		`- self.handlers = list(freshmaker.handlers.load_handlers())`
		`self.register_parsers()`
		`super(FreshmakerConsumer, self).__init__(hub)`

		`@@ -63,10 +58,9 @@`
		`self.incoming.put(msg)`

		`def register_parsers(self):`
		`- events.BaseEvent.register_parser(freshmaker.parsers.bodhi.BodhiUpdateCompleteStableParser)`
		`- events.BaseEvent.register_parser(freshmaker.parsers.git.GitReceiveParser)`
		`- events.BaseEvent.register_parser(freshmaker.parsers.koji.KojiTaskStateChangeParser)`
		`- events.BaseEvent.register_parser(freshmaker.parsers.mbs.MBSModuleStateChangeParser)`
		`+ parser_classes = load_classes(conf.parsers)`
		`+ for parser_class in parser_classes:`
		`+ events.BaseEvent.register_parser(parser_class)`
		`log.debug("Parser classes: %r", events.BaseEvent._parsers)`

		`self.topic = events.BaseEvent.get_parsed_topics()`
		`@@ -112,19 +106,19 @@`

		`def get_abstracted_msg(self, message):`
		`# Convert the message to an abstracted message`
		`- if conf.messaging == 'fedmsg' or conf.messaging == 'in_memory':`
		`- msg = events.BaseEvent.from_fedmsg(`
		`- message['topic'], message)`
		`- else:`
		`- raise ValueError('The messaging format "{0}" is not supported'`
		`- .format(conf.messaging))`
		`- return msg`
		`+ if 'topic' not in message:`
		`+ raise ValueError(`
		`+ 'The messaging format "{}" is not supported'.format(conf.messaging))`
		`+`
		`+ return events.BaseEvent.from_fedmsg(message['topic'], message)`

		`def process_event(self, msg):`
		`log.debug('Received a message with an ID of "{0}" and of type "{1}"'`
		`.format(getattr(msg, 'msg_id', None), type(msg).__name__))`

		`- for handler in self.handlers:`
		`+ for handler_class in load_classes(conf.handlers):`
		`+ handler = handler_class()`
		`+`
		`if not handler.can_handle(msg):`
		`continue`

freshmaker/errata.py

file modified

+2 -2

		`@@ -24,7 +24,7 @@`
		`import requests`
		`from requests_kerberos import HTTPKerberosAuth`

		`- from freshmaker.events import BrewRPMSignEvent`
		`+ from freshmaker.events import BrewSignRPMEvent`


		`class ErrataAdvisory(object):`
		`@@ -87,7 +87,7 @@`
		`:return: List of ErrataAdvisory instances`
		`:rtype: list`
		`"""`
		`- if isinstance(event, BrewRPMSignEvent):`
		`+ if isinstance(event, BrewSignRPMEvent):`
		`build = self._errata_rest_get("/build/%s" % str(event.nvr))`
		`if "all_errata" not in build:`
		`return []`

freshmaker/events.py

file modified

+2 -2

		`@@ -249,12 +249,12 @@`
		`self.task_state = task_state`


		`- class BrewRPMSignEvent(BaseEvent):`
		`+ class BrewSignRPMEvent(BaseEvent):`
		`"""`
		`Represents the message sent by Brew when RPM is signed.`
		`"""`
		`def __init__(self, msg_id, nvr):`
		`- super(BrewRPMSignEvent, self).__init__(msg_id)`
		`+ super(BrewSignRPMEvent, self).__init__(msg_id)`
		`self.nvr = nvr`

		`@property`

freshmaker/handlers/__init__.py

file modified

-23

		`@@ -23,35 +23,12 @@`

		`import abc`
		`import re`
		`- import sys`

		`from freshmaker import conf, log, db, models`
		`from freshmaker.mbs import MBS`
		`from freshmaker.kojiservice import koji_service`


		`- def load_class(location):`
		`- """ Take a string of the form 'fedmsg.consumers.ircbot:IRCBotConsumer'`
		`- and return the IRCBotConsumer class.`
		`- """`
		`- mod_name, cls_name = location.strip().split(':')`
		`-`
		`- __import__(mod_name)`
		`-`
		`- try:`
		`- return getattr(sys.modules[mod_name], cls_name)`
		`- except AttributeError:`
		`- raise ImportError("%r not found in %r" % (cls_name, mod_name))`
		`-`
		`-`
		`- def load_handlers():`
		`- """ Import and instantiate all handlers listed in the given config. """`
		`- for import_path in conf.handlers:`
		`- cls = load_class(import_path)`
		`- handler = cls()`
		`- yield handler`
		`-`
		`-`
		`class BaseHandler(object):`
		`"""`
		`Abstract base class for event handlers.`

freshmaker/parsers/brew/__init__.py

file added

empty or binary file added

freshmaker/parsers/brew/sign_rpm.py

file added

+41

		`@@ -0,0 +1,41 @@`
		`+ # -- coding: utf-8 --`
		`+ # Copyright (c) 2016 Red Hat, Inc.`
		`+ #`
		`+ # Permission is hereby granted, free of charge, to any person obtaining a copy`
		`+ # of this software and associated documentation files (the "Software"), to deal`
		`+ # in the Software without restriction, including without limitation the rights`
		`+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell`
		`+ # copies of the Software, and to permit persons to whom the Software is`
		`+ # furnished to do so, subject to the following conditions:`
		`+ #`
		`+ # The above copyright notice and this permission notice shall be included in all`
		`+ # copies or substantial portions of the Software.`
		`+ #`
		`+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR`
		`+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,`
		`+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE`
		`+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER`
		`+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,`
		`+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE`
		`+ # SOFTWARE.`
		`+ #`
		`+ # Written by Chenxiong Qi <cqi@redhat.com>`
		`+`
		`+ from freshmaker.parsers import BaseParser`
		`+ from freshmaker.events import BrewSignRPMEvent`
		`+`
		`+`
		`+ class BrewSignRpmParser(BaseParser):`
		`+ """Parser parsing message from Brew"""`
		`+`
		`+ name = "BrewSignRpmParser"`
		`+ topic_suffixes = ["eng.brew.sign.rpm"]`
		`+`
		`+ def can_parse(self, topic, msg):`
		`+ return any([topic.endswith(s) for s in self.topic_suffixes])`
		`+`
		`+ def parse(self, topic, msg):`
		`+ msg_id = msg.get('msg_id')`
		`+ msg_inner_msg = msg.get('msg')`
		`+`
		`+ return BrewSignRPMEvent(msg_id, msg_inner_msg['build']['nvr'])`

freshmaker/utils.py

file modified

+23

		`@@ -27,12 +27,35 @@`
		`import os`
		`import shutil`
		`import subprocess`
		`+ import sys`
		`import tempfile`
		`import time`

		`from freshmaker import conf`


		`+ def load_class(location):`
		`+ """ Take a string of the form 'fedmsg.consumers.ircbot:IRCBotConsumer'`
		`+ and return the IRCBotConsumer class.`
		`+ """`
		`+ try:`
		`+ mod_name, cls_name = location.strip().split(':')`
		`+ except ValueError:`
		`+ raise ImportError('Invalid import path.')`
		`+`
		`+ __import__(mod_name)`
		`+`
		`+ try:`
		`+ return getattr(sys.modules[mod_name], cls_name)`
		`+ except AttributeError:`
		`+ raise ImportError("%r not found in %r" % (cls_name, mod_name))`
		`+`
		`+`
		`+ def load_classes(import_paths):`
		`+ """Load classes from given paths"""`
		`+ return [load_class(import_path) for import_path in import_paths]`
		`+`
		`+`
		`def retry(timeout=conf.net_timeout, interval=conf.net_retry_interval, wait_on=Exception, logger=None):`
		`"""A decorator that allows to retry a section of code until success or timeout."""`
		`def wrapper(function):`

tests/test_consumer.py

file modified

+41 -15

		`@@ -18,19 +18,25 @@`
		`# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE`
		`# SOFTWARE.`

		`- import unittest`
		`- import mock`
		`import fedmsg.config`
		`+ import mock`
		`+ import unittest`

		`import freshmaker`

		`+ from freshmaker.events import BrewSignRPMEvent`

		`- class ConsumerTest(unittest.TestCase):`
		`- def setUp(self):`
		`- pass`

		`- def tearDown(self):`
		`- pass`
		`+ class ConsumerBaseTest(unittest.TestCase):`
		`+`
		`+ def _create_consumer(self):`
		`+ hub = mock.MagicMock()`
		`+ hub.config = fedmsg.config.load_config()`
		`+ hub.config['freshmakerconsumer'] = True`
		`+ return freshmaker.consumer.FreshmakerConsumer(hub)`
		`+`
		`+`
		`+ class ConsumerTest(ConsumerBaseTest):`

		`@mock.patch("freshmaker.handlers.mbs.module_state_change.MBSModuleStateChangeHandler.handle")`
		`@mock.patch("freshmaker.consumer.get_global_consumer")`
		`@@ -40,10 +46,7 @@`
		`to proper handler and is able to get the further work from`
		`the handler.`
		`"""`
		`- hub = mock.MagicMock()`
		`- hub.config = fedmsg.config.load_config()`
		`- hub.config['freshmakerconsumer'] = True`
		`- consumer = freshmaker.consumer.FreshmakerConsumer(hub)`
		`+ consumer = self._create_consumer()`
		`global_consumer.return_value = consumer`

		`msg = {'body': {`
		`@@ -68,14 +71,37 @@`
		`"""`
		`Tests consumer will try to subscribe specified topics.`
		`"""`
		`- hub = mock.MagicMock()`
		`- hub.config = fedmsg.config.load_config()`
		`- consumer = freshmaker.consumer.FreshmakerConsumer(hub)`
		`+ consumer = self._create_consumer()`
		`global_consumer.return_value = consumer`
		`topics = freshmaker.events.BaseEvent.get_parsed_topics()`
		`callback = consumer._consume_json if consumer.jsonify else consumer.consume`
		`for topic in topics:`
		`- self.assertIn(mock.call(topic, callback), hub.subscribe.call_args_list)`
		`+ self.assertIn(mock.call(topic, callback), consumer.hub.subscribe.call_args_list)`
		`+`
		`+`
		`+ class ParseBrewSignRPMEventTest(ConsumerBaseTest):`
		`+`
		`+ @mock.patch('freshmaker.events.conf.parsers',`
		`+ new=['freshmaker.parsers.brew.sign_rpm:BrewSignRpmParser'])`
		`+ @mock.patch("freshmaker.consumer.get_global_consumer")`
		`+ def test_get_internal_event_parser(self, get_global_consumer):`
		`+ consumer = self._create_consumer()`
		`+ get_global_consumer.return_value = consumer`
		`+`
		`+ msg = {`
		`+ 'msg_id': 'fake-msg-id',`
		`+ 'topic': '/topic/VirtualTopic.eng.brew.sign.rpm',`
		`+ 'msg': {`
		`+ 'build': {`
		`+ 'id': 562101,`
		`+ 'nvr': 'openshift-ansible-3.3.1.32-1.git.0.3b74dea.el7',`
		`+ }`
		`+ }`
		`+ }`
		`+ msg = consumer.get_abstracted_msg(msg)`
		`+ self.assertIsInstance(msg, BrewSignRPMEvent)`
		`+ self.assertEqual('fake-msg-id', msg.msg_id)`
		`+ self.assertEqual('openshift-ansible-3.3.1.32-1.git.0.3b74dea.el7', msg.nvr)`


		`if __name__ == '__main__':`

tests/test_errata.py

file modified

+3 -3

		`@@ -25,7 +25,7 @@`
		`from mock import patch`

		`from freshmaker.errata import Errata`
		`- from freshmaker.events import BrewRPMSignEvent, GitRPMSpecChangeEvent`
		`+ from freshmaker.events import BrewSignRPMEvent, GitRPMSpecChangeEvent`


		`class MockedErrataRESTAPI(object):`
		`@@ -81,7 +81,7 @@`
		`@patch.object(Errata, "_errata_rest_get")`
		`def test_advisories_from_event(self, errata_rest_get):`
		`MockedErrataRESTAPI(errata_rest_get)`
		`- event = BrewRPMSignEvent("msgid", "libntirpc-1.4.3-4.el7rhgs")`
		`+ event = BrewSignRPMEvent("msgid", "libntirpc-1.4.3-4.el7rhgs")`
		`advisories = self.errata.advisories_from_event(event)`
		`self.assertEqual(len(advisories), 1)`
		`self.assertEqual(advisories[0].errata_id, 28484)`
		`@@ -91,7 +91,7 @@`
		`mocked_errata = MockedErrataRESTAPI(errata_rest_get)`
		`del mocked_errata.builds["libntirpc-1.4.3-4.el7rhgs"]["all_errata"]`

		`- event = BrewRPMSignEvent("msgid", "libntirpc-1.4.3-4.el7rhgs")`
		`+ event = BrewSignRPMEvent("msgid", "libntirpc-1.4.3-4.el7rhgs")`
		`advisories = self.errata.advisories_from_event(event)`
		`self.assertEqual(len(advisories), 0)`

cqi commented 8 years ago

This patch introduces an environment variable to switch freshmaker to an
internal mode to parse and handle messages from UMB specifically. So, in
configuration, all parsers and handlers for handling messages from
Fedora fedmsg are disabled when deploy internally and enable the mode.

FRESHMAKER_INSIDE_RH, set it to enable internal mode whatever its
value is.

Configure fedmsg-hub to use STOMP to connect to UMB and receive
messages. Broker URLs, certificate and private key files can be
configured via several new FRESHMAKER_STOMP_* environment variables.

FRESHMAKER_STOMP_URI, URIs of brokers separated by comma.
FRESHMAKER_STOMP_SSL_CRT, path to certificate file.
FRESHMAKER_STOMP_SSL_KEY, path to private key file.
FRESHMAKER_MESSAGING_TOPIC_PREFIX, part of queue name from the
beginning to word VirtualTopic, e.g.
/queue/Consumer.msg-client-me.freshmaker_queue.VirtualTopic

Major changes:

Make event parsers configurable along with handlers in config
Reuse load_classes to load event parsers and handlers from config
Modify several config to add internal configuration for event parsers
and handlers
BrewRPMSignEvent is renamed to BrewSignRPMEvent, which is natrual
to follow the topic name schema brew.sign.rpm
Add event parse for BrewSignRPMEvent

Signed-off-by: Chenxiong Qi cqi@redhat.com

rebased

8 years ago

rebased

8 years ago

rebased

8 years ago

jkaluza commented on line 12 of freshmaker/consumer.py 8 years ago

+1 for this!

rebased

8 years ago

cqi commented 8 years ago

Add test.

Ready for review.

jkaluza commented 8 years ago

+1 here, I will wait for the Monday if qwan wants to review that. Feel free to merge on Monday :)

qwan commented on line 13 of conf/config.py 8 years ago

This looks not so reasonable, can we just use something like ZMQ_ENABLED and STOMP_ENABLED to enable/disable connecting to zmq and stomp endpoints?

qwan commented on line 40 of conf/config.py 8 years ago

I think it's unnecessary to change the parsers & handlers in this way, when you need to disable//enable some of the parsers/handlers, you can just edit the config options directly.

qwan commented on line 45 of conf/config.py 8 years ago

I'd prefer to just use PARSERS.

qwan commented on line 80 of conf/config.py 8 years ago

Same as above, we can edit this option directly rather than by checking a variable.

qwan commented on line 13 of conf/config.py 8 years ago

something like:
if ZMQ_ENABLED and STOMP_ENABLED:
# raise error if we can't support enabling ZMQ and STOMP in the same time.
if ZMQ_ENABLED:
# ZMQ specific options
if STOMP_ENABLED:
# STOMP specific options.

qwan commented on line 11 of fedmsg.d/freshmaker.py 8 years ago

I think we need to support specifying the options in config rather than only support getting them from os.environ.

cqi commented on line 11 of fedmsg.d/freshmaker.py 8 years ago

I guess you mean freshmaker's config. How to read freshmaker's config from this file?

Reading these stomp configuration from a config file could be good, but, how much extra benefit to freshmaker in practice?

This file should be read by fedmsg (or probably the fedmsg-hub, I'm not sure) to configure itself. Using environment variables is a convenient way during development normally. It could also be used to configure freshmaker when deploy and run in a server, but in practice, these stomp_* options should be replaced with real value by some deployment scripts, e.g. ansible playbook.

Whatever the way to read options from freshmaker config file, this file is a fedmsg-hub configuration file, I think it is not a good idea to introduce a dependency between fedmsg-hub and freshmaker in the aspect of configuration.

qwan commented on line 11 of fedmsg.d/freshmaker.py 8 years ago

This fedmsg.d/freshmaker.py is just the config I mean, which we'll put it under /etc/fedmsg.d while deploying, yes it's easy to have the variables configured with ansible scripts, but it's not a good idea to assume we will always use ansible for deploying. It's good to have it be configurable via environment variables, but not good to unable to configure in this config file.

qwan commented on line 13 of conf/config.py 8 years ago

Sorry, the comments are for the change in fedmsg.d/freshmaker.py. In my opinion, RH_MODE and FRESHMAKER_INSIDE_RH can just be removed. it makes sense to just update MESSAGING_TOPIC_PREFIX, HANDLERS and PARSERS directly in this config when necessary.

jkaluza commented on line 13 of conf/config.py 8 years ago

We would then need to have two config.py files in the repository, one for RH and one for Fedora. I don't want to configure all the fields in config.py to have Freshmaker working on Fedora/RH when I need to test something :).

cqi commented on line 13 of conf/config.py 8 years ago

Separate config file for RH is my original thought. However, a bug in init_config need to fix in advance, that is freshmaker does not load and read config from FRESHMAKER_CONFIG_FILE when specify FRESHMAKER_DEVELOPER_ENV.

rebased

8 years ago

2 new commits added

Split config files
Connect to UMB and parse brew.sign.rpm

8 years ago

2 new commits added

Split config files
Connect to UMB and parse brew.sign.rpm

8 years ago

2 new commits added

Split config files
Connect to UMB and parse brew.sign.rpm

8 years ago

qwan commented on line 22 of conf/config.py 8 years ago

What's your opinion on renaming this to just PARSERS?

qwan commented on line 1 of conf/configrh.py 8 years ago

Compare to config.py, the difference is configuring message topic and disable/enable some handlers/parsers, I personally think it's easy to just update conf/config.py when you need to deploy Freshmaker in different env, I'm wondering does it worth to add another config file for this case.

qwan commented 8 years ago

And I'd suggest squash the 2 commits since the second one is just reverting the changes in the first one with minor updates.

rebased

8 years ago

cqi commented 8 years ago

Commits are squashed. EVENT_PARSERS is renamed to PARSERS.

cqi commented 8 years ago