)]}'
{"mistral/messaging.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"3cbefa83180317ba5c6f7d70fd2b9add163dd288","unresolved":false,"context_lines":[{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"def event_handler(self, ctxt, publisher_id, event_type, payload, metadata):"},{"line_number":35,"context_line":"    LOG.debug(\u0027Received notification. publisher_id: %s, event_type: %s, \u0027"},{"line_number":36,"context_line":"              \u0027payload: %s\u0027, publisher_id, event_type, payload)"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_bc73d614","line":34,"range":{"start_line":34,"start_character":4,"end_line":34,"end_character":17},"updated":"2016-07-08 10:25:56.000000000","message":"A few questions about this method:\n\n* Methods are usually called as verbs (do_this, do_that, does_that), not as nouns. Why did you choose this name?\n* Why does it have \u0027self\u0027 parameter whereas it\u0027s not a member, it\u0027s a function?\n* Where is it used?","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"ee71c9a627a754536c2e25dc0c7df9bf48ae071c","unresolved":false,"context_lines":[{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"def event_handler(self, ctxt, publisher_id, event_type, payload, metadata):"},{"line_number":35,"context_line":"    LOG.debug(\u0027Received notification. publisher_id: %s, event_type: %s, \u0027"},{"line_number":36,"context_line":"              \u0027payload: %s\u0027, publisher_id, event_type, payload)"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_a510284f","line":34,"range":{"start_line":34,"start_character":4,"end_line":34,"end_character":17},"in_reply_to":"1aa78d24_bc73d614","updated":"2016-07-12 23:47:35.000000000","message":"hi, Renat,\n\n* good suggestion for the function name.\n* the function is used in _get_endpoint_cls method in services/event_engine.py when constructing the listener endpoint.","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"3cbefa83180317ba5c6f7d70fd2b9add163dd288","unresolved":false,"context_lines":[{"line_number":36,"context_line":"              \u0027payload: %s\u0027, publisher_id, event_type, payload)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    try:"},{"line_number":39,"context_line":"        self.engine.process_notification_event(ctxt, event_type, payload,"},{"line_number":40,"context_line":"                                               metadata)"},{"line_number":41,"context_line":"        return dispatcher.NotificationResult.HANDLED"},{"line_number":42,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_7c916e52","line":39,"range":{"start_line":39,"start_character":47,"end_line":39,"end_character":51},"updated":"2016-07-08 10:25:56.000000000","message":"soft:\n\nWe usually use form\n\nmtd(\n    param1,\n    param2,\n    ...\n)","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"ee71c9a627a754536c2e25dc0c7df9bf48ae071c","unresolved":false,"context_lines":[{"line_number":36,"context_line":"              \u0027payload: %s\u0027, publisher_id, event_type, payload)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    try:"},{"line_number":39,"context_line":"        self.engine.process_notification_event(ctxt, event_type, payload,"},{"line_number":40,"context_line":"                                               metadata)"},{"line_number":41,"context_line":"        return dispatcher.NotificationResult.HANDLED"},{"line_number":42,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_e544f047","line":39,"range":{"start_line":39,"start_character":47,"end_line":39,"end_character":51},"in_reply_to":"1aa78d24_7c916e52","updated":"2016-07-12 23:47:35.000000000","message":"Done","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"3cbefa83180317ba5c6f7d70fd2b9add163dd288","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    :param: exchange exchange name"},{"line_number":71,"context_line":"    \"\"\""},{"line_number":72,"context_line":"    pool_name \u003d \u0027mistral-%s-%s\u0027 % (exchange, socket.gethostname())"},{"line_number":73,"context_line":"    LOG.debug(\"Listener pool name is %s\", pool_name)"},{"line_number":74,"context_line":"    return pool_name"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"def start_listener(conf, exchange, topic, endpoints):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_3cd526b9","line":74,"range":{"start_line":72,"start_character":3,"end_line":74,"end_character":20},"updated":"2016-07-08 10:25:56.000000000","message":"Soft: I would split all 3 lines with blank lines. These are logically different units.","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"ee71c9a627a754536c2e25dc0c7df9bf48ae071c","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    :param: exchange exchange name"},{"line_number":71,"context_line":"    \"\"\""},{"line_number":72,"context_line":"    pool_name \u003d \u0027mistral-%s-%s\u0027 % (exchange, socket.gethostname())"},{"line_number":73,"context_line":"    LOG.debug(\"Listener pool name is %s\", pool_name)"},{"line_number":74,"context_line":"    return pool_name"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"def start_listener(conf, exchange, topic, endpoints):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_4576a4a4","line":74,"range":{"start_line":72,"start_character":3,"end_line":74,"end_character":20},"in_reply_to":"1aa78d24_3cd526b9","updated":"2016-07-12 23:47:35.000000000","message":"Done","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"change_message_id":"2ec09c78d7625fbe36a85961c397afdcbc54467f","unresolved":false,"context_lines":[{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"def handle_event(self, ctxt, publisher_id, event_type, payload, metadata):"},{"line_number":35,"context_line":"    \"\"\"Handler of each priority of notification messages."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    The function is used to construct endpoint class dynamically when starting"}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_69b7b886","line":34,"range":{"start_line":34,"start_character":17,"end_line":34,"end_character":21},"updated":"2016-07-19 09:53:07.000000000","message":"I don\u0027t understand why self here ?","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"8460f7b99213aa5dd516ab1d5b46cc5506bb3112","unresolved":false,"context_lines":[{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"def handle_event(self, ctxt, publisher_id, event_type, payload, metadata):"},{"line_number":35,"context_line":"    \"\"\"Handler of each priority of notification messages."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    The function is used to construct endpoint class dynamically when starting"}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_2dccfe87","line":34,"range":{"start_line":34,"start_character":17,"end_line":34,"end_character":21},"in_reply_to":"dada55a8_69b7b886","updated":"2016-07-22 03:38:30.000000000","message":"As the docstring said, this method is actually a method defined in a class. \u0027self\u0027 here doesn\u0027t mean anything, but after the class is created, it will make sense.","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"e8919a42d876c6e4d89cf9912219d773a09e4ba5","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            metadata"},{"line_number":49,"context_line":"        )"},{"line_number":50,"context_line":"        return dispatcher.NotificationResult.HANDLED"},{"line_number":51,"context_line":"    except Exception:"},{"line_number":52,"context_line":"        pass"},{"line_number":53,"context_line":"    finally:"},{"line_number":54,"context_line":"        pass"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_2883f005","line":52,"range":{"start_line":51,"start_character":0,"end_line":52,"end_character":12},"updated":"2016-07-19 13:13:03.000000000","message":"It should at least log error","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"8460f7b99213aa5dd516ab1d5b46cc5506bb3112","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            metadata"},{"line_number":49,"context_line":"        )"},{"line_number":50,"context_line":"        return dispatcher.NotificationResult.HANDLED"},{"line_number":51,"context_line":"    except Exception:"},{"line_number":52,"context_line":"        pass"},{"line_number":53,"context_line":"    finally:"},{"line_number":54,"context_line":"        pass"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_2d1ebe0b","line":52,"range":{"start_line":51,"start_character":0,"end_line":52,"end_character":12},"in_reply_to":"dada55a8_2883f005","updated":"2016-07-22 03:38:30.000000000","message":"Hi, it was fixed in subsequent patch: https://review.openstack.org/#/c/320509/15/mistral/messaging.py","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"e8919a42d876c6e4d89cf9912219d773a09e4ba5","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        return dispatcher.NotificationResult.HANDLED"},{"line_number":51,"context_line":"    except Exception:"},{"line_number":52,"context_line":"        pass"},{"line_number":53,"context_line":"    finally:"},{"line_number":54,"context_line":"        pass"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"@six.add_metaclass(abc.ABCMeta)"}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_a8c9a07a","line":54,"range":{"start_line":53,"start_character":0,"end_line":54,"end_character":12},"updated":"2016-07-19 13:13:03.000000000","message":"this do nothing and can be safely deleted","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"change_message_id":"2ec09c78d7625fbe36a85961c397afdcbc54467f","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    targets \u003d [target.Target(exchange\u003dexchange, topic\u003dtopic)]"},{"line_number":100,"context_line":"    pool_name \u003d get_pool_name(exchange)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    mylistener \u003d listener.get_notification_listener("},{"line_number":103,"context_line":"        trans,"},{"line_number":104,"context_line":"        targets,"},{"line_number":105,"context_line":"        endpoints,"}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_1fa2aeae","line":102,"range":{"start_line":102,"start_character":4,"end_line":102,"end_character":14},"updated":"2016-07-19 09:53:07.000000000","message":"mylistener ? I think we should use some appropriate name.","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"}],"mistral/services/event_engine.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"2bf6388b73c1523760d9b7d3fe2b96785b471c7d","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                       the event trigger."},{"line_number":223,"context_line":"        \"\"\""},{"line_number":224,"context_line":"        with self.lock:"},{"line_number":225,"context_line":"            self.event_triggers_map[trigger[\u0027event\u0027]].append(trigger)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        self._add_event_listener(trigger[\u0027exchange\u0027], trigger[\u0027topic\u0027], events)"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_3c576b27","line":225,"range":{"start_line":225,"start_character":12,"end_line":225,"end_character":69},"updated":"2016-07-06 13:57:44.000000000","message":"It should be checked if given trigger already exist in event_triggers_map[trigger[\u0027event\u0027]], shouldn\u0027t it?","commit_id":"9b0cd0675a697d98af4507b5c3878e9d6ed53e2a"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"d1f010c120ec4efd0dc0c6468125c99b245ce161","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                       the event trigger."},{"line_number":223,"context_line":"        \"\"\""},{"line_number":224,"context_line":"        with self.lock:"},{"line_number":225,"context_line":"            self.event_triggers_map[trigger[\u0027event\u0027]].append(trigger)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        self._add_event_listener(trigger[\u0027exchange\u0027], trigger[\u0027topic\u0027], events)"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_b402e670","line":225,"range":{"start_line":225,"start_character":12,"end_line":225,"end_character":69},"in_reply_to":"1aa78d24_185512c5","updated":"2016-07-07 22:04:35.000000000","message":"no, the db design guarantees the duplicate won\u0027t happen, in other words, users can not create event triggers with same exchange+topic+event+workflow, please see the db migration script.","commit_id":"9b0cd0675a697d98af4507b5c3878e9d6ed53e2a"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"b299da356005b569e8aae2928cb9848572e1db3c","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                       the event trigger."},{"line_number":223,"context_line":"        \"\"\""},{"line_number":224,"context_line":"        with self.lock:"},{"line_number":225,"context_line":"            self.event_triggers_map[trigger[\u0027event\u0027]].append(trigger)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        self._add_event_listener(trigger[\u0027exchange\u0027], trigger[\u0027topic\u0027], events)"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_e70fcb34","line":225,"range":{"start_line":225,"start_character":12,"end_line":225,"end_character":69},"in_reply_to":"1aa78d24_3c576b27","updated":"2016-07-07 11:06:08.000000000","message":"Hi, I can see there is a possibility that it may duplicate, is there?","commit_id":"9b0cd0675a697d98af4507b5c3878e9d6ed53e2a"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"1683c668ae3d875bc96ee1397b762b8f23e13bb0","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                       the event trigger."},{"line_number":223,"context_line":"        \"\"\""},{"line_number":224,"context_line":"        with self.lock:"},{"line_number":225,"context_line":"            self.event_triggers_map[trigger[\u0027event\u0027]].append(trigger)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        self._add_event_listener(trigger[\u0027exchange\u0027], trigger[\u0027topic\u0027], events)"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_c2f7cb8a","line":225,"range":{"start_line":225,"start_character":12,"end_line":225,"end_character":69},"in_reply_to":"1aa78d24_b402e670","updated":"2016-07-08 15:54:53.000000000","message":"OK, I didn\u0027t see how the communication is working (first add to db, then send RPC request). Nevertheless, this is a part of public RPC API, so shouldn\u0027t we check this (I\u0027m not sure if we allowed another services to use our RPC API)? Also, message duplicates may happen if rabbitmq is in HA mode https://www.rabbitmq.com/reliability.html","commit_id":"9b0cd0675a697d98af4507b5c3878e9d6ed53e2a"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"26481920446d3034410589678c7d2a88bbedd82a","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                       the event trigger."},{"line_number":223,"context_line":"        \"\"\""},{"line_number":224,"context_line":"        with self.lock:"},{"line_number":225,"context_line":"            self.event_triggers_map[trigger[\u0027event\u0027]].append(trigger)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        self._add_event_listener(trigger[\u0027exchange\u0027], trigger[\u0027topic\u0027], events)"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_45b864c8","line":225,"range":{"start_line":225,"start_character":12,"end_line":225,"end_character":69},"in_reply_to":"1aa78d24_c2f7cb8a","updated":"2016-07-12 23:51:27.000000000","message":"Done","commit_id":"9b0cd0675a697d98af4507b5c3878e9d6ed53e2a"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"c8d34b7ca3c92732abe7b5b6fcc85fcec55a41c6","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                       the event trigger."},{"line_number":223,"context_line":"        \"\"\""},{"line_number":224,"context_line":"        with self.lock:"},{"line_number":225,"context_line":"            self.event_triggers_map[trigger[\u0027event\u0027]].append(trigger)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        self._add_event_listener(trigger[\u0027exchange\u0027], trigger[\u0027topic\u0027], events)"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_185512c5","line":225,"range":{"start_line":225,"start_character":12,"end_line":225,"end_character":69},"in_reply_to":"1aa78d24_e70fcb34","updated":"2016-07-07 11:46:01.000000000","message":"From what I understand event_triggers are created by user, therefore user may send same request twice, which will result in duplicating.","commit_id":"9b0cd0675a697d98af4507b5c3878e9d6ed53e2a"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"3cbefa83180317ba5c6f7d70fd2b9add163dd288","unresolved":false,"context_lines":[{"line_number":29,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":30,"context_line":"CONF \u003d cfg.CONF"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"# Event queue event constants"},{"line_number":33,"context_line":"EVENT_CONTEXT \u003d \u0027context\u0027"},{"line_number":34,"context_line":"EVENT_TYPE \u003d \u0027type\u0027"},{"line_number":35,"context_line":"EVENT_PAYLOAD \u003d \u0027payload\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_bc7d96b3","line":32,"updated":"2016-07-08 10:25:56.000000000","message":"Soft: missing period at the end of the comment (please check this in all files).","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"ee71c9a627a754536c2e25dc0c7df9bf48ae071c","unresolved":false,"context_lines":[{"line_number":29,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":30,"context_line":"CONF \u003d cfg.CONF"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"# Event queue event constants"},{"line_number":33,"context_line":"EVENT_CONTEXT \u003d \u0027context\u0027"},{"line_number":34,"context_line":"EVENT_TYPE \u003d \u0027type\u0027"},{"line_number":35,"context_line":"EVENT_PAYLOAD \u003d \u0027payload\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_e55b5020","line":32,"in_reply_to":"1aa78d24_bc7d96b3","updated":"2016-07-12 23:47:35.000000000","message":"Done","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"3cbefa83180317ba5c6f7d70fd2b9add163dd288","unresolved":false,"context_lines":[{"line_number":36,"context_line":"EVENT_METADATA \u003d \u0027metadata\u0027"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class EventEngine(coordination.Service):"},{"line_number":40,"context_line":"    def __init__(self, engine_client):"},{"line_number":41,"context_line":"        coordination.Service.__init__(self, \u0027event_engine_group\u0027)"},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_bc2bd693","line":39,"range":{"start_line":39,"start_character":6,"end_line":39,"end_character":17},"updated":"2016-07-08 10:25:56.000000000","message":"I\u0027d suggest you write a couple of words about the service in docstring.","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"ee71c9a627a754536c2e25dc0c7df9bf48ae071c","unresolved":false,"context_lines":[{"line_number":36,"context_line":"EVENT_METADATA \u003d \u0027metadata\u0027"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class EventEngine(coordination.Service):"},{"line_number":40,"context_line":"    def __init__(self, engine_client):"},{"line_number":41,"context_line":"        coordination.Service.__init__(self, \u0027event_engine_group\u0027)"},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_45bce4db","line":39,"range":{"start_line":39,"start_character":6,"end_line":39,"end_character":17},"in_reply_to":"1aa78d24_bc2bd693","updated":"2016-07-12 23:47:35.000000000","message":"Done","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"3cbefa83180317ba5c6f7d70fd2b9add163dd288","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        }"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        LOG.debug(\"Adding notification event to event queue: %s\", event)"},{"line_number":212,"context_line":"        self.event_queue.put(event)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def create_event_trigger(self, trigger, events):"},{"line_number":215,"context_line":"        \"\"\"An endpoint method for creating event trigger."}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_ff2f103c","line":212,"range":{"start_line":212,"start_character":8,"end_line":212,"end_character":12},"updated":"2016-07-08 10:25:56.000000000","message":"Soft: a blank line after logging.","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"ee71c9a627a754536c2e25dc0c7df9bf48ae071c","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        }"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        LOG.debug(\"Adding notification event to event queue: %s\", event)"},{"line_number":212,"context_line":"        self.event_queue.put(event)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def create_event_trigger(self, trigger, events):"},{"line_number":215,"context_line":"        \"\"\"An endpoint method for creating event trigger."}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_25bf18d1","line":212,"range":{"start_line":212,"start_character":8,"end_line":212,"end_character":12},"in_reply_to":"1aa78d24_ff2f103c","updated":"2016-07-12 23:47:35.000000000","message":"Done","commit_id":"a079e7abebb7132af85fe6bb836ed578f430d2b0"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"e8919a42d876c6e4d89cf9912219d773a09e4ba5","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        )"},{"line_number":74,"context_line":"        attrs[\u0027event_types\u0027] \u003d events"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        endpoint_cls \u003d type("},{"line_number":77,"context_line":"            \u0027MistralNotificationEndpoint\u0027,"},{"line_number":78,"context_line":"            (mistral_messaging.NotificationEndpoint,),"},{"line_number":79,"context_line":"            attrs,"},{"line_number":80,"context_line":"        )"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"        return endpoint_cls"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_28a01043","line":80,"range":{"start_line":76,"start_character":0,"end_line":80,"end_character":9},"updated":"2016-07-19 13:13:03.000000000","message":"There is no logic in this method that would result in different meta-classes. So why not just create new class like:\n\n class MistralNotificationEndpoint(NotificationEndpoint):\n  def audit(self, **kwargs):\n   return self._handle_events(**kwargs)\n  def critical(self, **kwargs):\n   return self._handle_events(**kwargs)\n  def debug(self, **kwargs):\n   return self._handle_events(**kwargs)\n  def error(self, **kwargs):\n   return self._handle_events(**kwargs)\n  def info(self, **kwargs):\n   return self._handle_events(**kwargs)\n\nit will have \u0027event_types\u0027 as input parameter for the constructor and method _handle_events with same body as mistral/messaging/handle_event. \n\nIMO such approach would be easier to understand and to debug in the future. But of course, I may miss why meta-class is needed in this case.","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"8460f7b99213aa5dd516ab1d5b46cc5506bb3112","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        )"},{"line_number":74,"context_line":"        attrs[\u0027event_types\u0027] \u003d events"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        endpoint_cls \u003d type("},{"line_number":77,"context_line":"            \u0027MistralNotificationEndpoint\u0027,"},{"line_number":78,"context_line":"            (mistral_messaging.NotificationEndpoint,),"},{"line_number":79,"context_line":"            attrs,"},{"line_number":80,"context_line":"        )"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"        return endpoint_cls"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_4d70520b","line":80,"range":{"start_line":76,"start_character":0,"end_line":80,"end_character":9},"in_reply_to":"dada55a8_28a01043","updated":"2016-07-22 03:38:30.000000000","message":"you see, the duplicate code is the exact reason that I don\u0027t like the way you suggested. but, thanks anyway.","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"a9cc0821fca2f88be70aaf56258c89986c6c8d4e","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        )"},{"line_number":74,"context_line":"        attrs[\u0027event_types\u0027] \u003d events"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        endpoint_cls \u003d type("},{"line_number":77,"context_line":"            \u0027MistralNotificationEndpoint\u0027,"},{"line_number":78,"context_line":"            (mistral_messaging.NotificationEndpoint,),"},{"line_number":79,"context_line":"            attrs,"},{"line_number":80,"context_line":"        )"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"        return endpoint_cls"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_9d66f327","line":80,"range":{"start_line":76,"start_character":0,"end_line":80,"end_character":9},"in_reply_to":"dada55a8_4d70520b","updated":"2016-07-22 08:25:41.000000000","message":"According to pep20[1]: \u0027Explicit is better than implicit\u0027. With my suggestion it would be obvious for someone who would be reading this code what public and private methods MistralNotificationEndpoint has. With meta-clasess, on the other hand, it requires some attention to understand what is happening.\n\n[1] https://www.python.org/dev/peps/pep-0020/","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"7d4bd464394a6e62a2d471184f34b530589ab8a9","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        )"},{"line_number":74,"context_line":"        attrs[\u0027event_types\u0027] \u003d events"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        endpoint_cls \u003d type("},{"line_number":77,"context_line":"            \u0027MistralNotificationEndpoint\u0027,"},{"line_number":78,"context_line":"            (mistral_messaging.NotificationEndpoint,),"},{"line_number":79,"context_line":"            attrs,"},{"line_number":80,"context_line":"        )"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"        return endpoint_cls"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_81732515","line":80,"range":{"start_line":76,"start_character":0,"end_line":80,"end_character":9},"in_reply_to":"dada55a8_6171d177","updated":"2016-07-22 10:59:43.000000000","message":"There is one important difference between your code and the one from link: the action factory results in returning different classes depending on the input, whereas your code always returns same class.","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"68700a3e69d295a24662d7ed628f78f26590004b","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        )"},{"line_number":74,"context_line":"        attrs[\u0027event_types\u0027] \u003d events"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        endpoint_cls \u003d type("},{"line_number":77,"context_line":"            \u0027MistralNotificationEndpoint\u0027,"},{"line_number":78,"context_line":"            (mistral_messaging.NotificationEndpoint,),"},{"line_number":79,"context_line":"            attrs,"},{"line_number":80,"context_line":"        )"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"        return endpoint_cls"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_6171d177","line":80,"range":{"start_line":76,"start_character":0,"end_line":80,"end_character":9},"in_reply_to":"dada55a8_9d66f327","updated":"2016-07-22 10:52:30.000000000","message":"I agree with what you said and the docs, but I really don\u0027t think the code here is explicit enough, you can see the similar code here: https://github.com/openstack/mistral/blob/master/mistral/actions/action_factory.py\n\nI will stay here to see others\u0027 opinion to see if it deserves another patchset to make us happy :-)","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"e8919a42d876c6e4d89cf9912219d773a09e4ba5","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        if key in self.exchange_topic_listener_map:"},{"line_number":95,"context_line":"            listener \u003d self.exchange_topic_listener_map[key]"},{"line_number":96,"context_line":"            listener.stop()"},{"line_number":97,"context_line":"            listener.wait()"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        endpoint \u003d self._get_endpoint_cls(events)(self)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_2b45aa6f","line":97,"range":{"start_line":97,"start_character":12,"end_line":97,"end_character":27},"updated":"2016-07-19 13:13:03.000000000","message":"Why there is a need to call listener.wait() after it was stopped?","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"8460f7b99213aa5dd516ab1d5b46cc5506bb3112","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        if key in self.exchange_topic_listener_map:"},{"line_number":95,"context_line":"            listener \u003d self.exchange_topic_listener_map[key]"},{"line_number":96,"context_line":"            listener.stop()"},{"line_number":97,"context_line":"            listener.wait()"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        endpoint \u003d self._get_endpoint_cls(events)(self)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_adf3ce71","line":97,"range":{"start_line":97,"start_character":12,"end_line":97,"end_character":27},"in_reply_to":"dada55a8_2b45aa6f","updated":"2016-07-22 03:38:30.000000000","message":"After calling stop(), there may still be some existing messages which have not been completely processed. The wait() method blocks until all message processing has completed.","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"a9cc0821fca2f88be70aaf56258c89986c6c8d4e","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        if key in self.exchange_topic_listener_map:"},{"line_number":95,"context_line":"            listener \u003d self.exchange_topic_listener_map[key]"},{"line_number":96,"context_line":"            listener.stop()"},{"line_number":97,"context_line":"            listener.wait()"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        endpoint \u003d self._get_endpoint_cls(events)(self)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dada55a8_3d5d4749","line":97,"range":{"start_line":97,"start_character":12,"end_line":97,"end_character":27},"in_reply_to":"dada55a8_adf3ce71","updated":"2016-07-22 08:25:41.000000000","message":"Thanks for the explanation.","commit_id":"4eec8e63ab21ac05d44db67054534b6b6f854fa4"},{"author":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"change_message_id":"be6936758184db8d41b52e702245784ca52730be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"dada55a8_5e459a40","updated":"2016-07-22 05:19:40.000000000","message":"I think, mistral/engine is better place for this file. thoughts ?","commit_id":"733bf44fe59d53b722a853a7dd3e62973509714a"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"2c7efa6172892895af9572c67521682cd0b8d0a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"dada55a8_cfb391e8","in_reply_to":"dada55a8_5e459a40","updated":"2016-07-22 07:44:30.000000000","message":"I think no, because engine/ contains all core functionality for Mistral Engine which takes care of workflow execution. This event engine is an additional extension that allows to listen to events and trigger workflows. I think it\u0027s ok to have it under services/","commit_id":"733bf44fe59d53b722a853a7dd3e62973509714a"}]}