)]}'
{"oslo_messaging/_drivers/base.py":[{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"ab81089fe4ac51ce68e5850e95ccbbdf7f3a61b3","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        self._poll_style_listener.stop()"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def wait(self):"},{"line_number":225,"context_line":"        self._listen_thread.join()"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def cleanup(self):"},{"line_number":228,"context_line":"        \"\"\"Cleanup listener."}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_576891f5","side":"PARENT","line":225,"updated":"2016-04-18 09:00:09.000000000","message":"This change a bit the public API behavior, it would be could, to have that into a separate patch, so if we discover a bad thing due to this, we can revert only this part instead of the whole change.","commit_id":"0b286754e22760b4589fd2a33e1ebad2318d2bce"},{"author":{"_account_id":8601,"name":"Dmitriy Ukhlov","email":"dukhlov@mirantis.com","username":"dukhlov"},"change_message_id":"1911fe563e0c0ad6bde8e29d95c458c629932692","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        self._poll_style_listener.stop()"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def wait(self):"},{"line_number":225,"context_line":"        self._listen_thread.join()"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def cleanup(self):"},{"line_number":228,"context_line":"        \"\"\"Cleanup listener."}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_ea08cfdb","side":"PARENT","line":225,"in_reply_to":"1a122d0e_576891f5","updated":"2016-04-18 10:14:53.000000000","message":"new listener interface was added a few weeks ago so we shouldn\u0027t have problems with public API change, it is new and not used by third part driver yet. I prefer to leave this change in this patch but will remove if you insist","commit_id":"0b286754e22760b4589fd2a33e1ebad2318d2bce"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"0fbdaa9dd0078cc10f8b784ddbd902bc027ce564","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        self._poll_style_listener.stop()"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def wait(self):"},{"line_number":225,"context_line":"        self._listen_thread.join()"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def cleanup(self):"},{"line_number":228,"context_line":"        \"\"\"Cleanup listener."}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_602cb202","side":"PARENT","line":225,"in_reply_to":"1a122d0e_ea08cfdb","updated":"2016-04-19 08:18:37.000000000","message":"No we can keep it, this is clearly a good idea, it\u0027s just a general advise for future review.","commit_id":"0b286754e22760b4589fd2a33e1ebad2318d2bce"}],"oslo_messaging/notify/listener.py":[{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"1755f65b004914c0d118d65c42410b0366257d96","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        try:"},{"line_number":150,"context_line":"            res \u003d self.dispatcher.dispatch(incoming)"},{"line_number":151,"context_line":"        except Exception:"},{"line_number":152,"context_line":"            LOG.exception(_LE(\u0027Exception during handling message: %s:\u0027),"},{"line_number":153,"context_line":"                          incoming)"},{"line_number":154,"context_line":"            res \u003d notify_dispatcher.NotificationResult.REQUEUE"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_37f355a0","line":152,"updated":"2016-04-18 09:00:07.000000000","message":"This changes the error log, this is not refactoring.","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"68da464f806848c3bb377d1678c757fc6c21245d","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        try:"},{"line_number":150,"context_line":"            res \u003d self.dispatcher.dispatch(incoming)"},{"line_number":151,"context_line":"        except Exception:"},{"line_number":152,"context_line":"            LOG.exception(_LE(\u0027Exception during handling message: %s:\u0027),"},{"line_number":153,"context_line":"                          incoming)"},{"line_number":154,"context_line":"            res \u003d notify_dispatcher.NotificationResult.REQUEUE"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_db668b28","line":152,"in_reply_to":"1a122d0e_37f355a0","updated":"2016-04-18 09:01:49.000000000","message":"This is a nit, but increase the patch size why it could be into a separate easy to review change.","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"1755f65b004914c0d118d65c42410b0366257d96","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            else:"},{"line_number":161,"context_line":"                incoming.acknowledge()"},{"line_number":162,"context_line":"        except Exception:"},{"line_number":163,"context_line":"            LOG.exception(_LE(\"Fail to ack/requeue message: %s\"), incoming)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"class BatchNotificationServer(NotificationServerBase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_b7fe4569","line":163,"updated":"2016-04-18 09:00:07.000000000","message":"This changes the error log, this is not refactoring.","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"1755f65b004914c0d118d65c42410b0366257d96","unresolved":false,"context_lines":[{"line_number":170,"context_line":"            not_processed_messages \u003d self.dispatcher.dispatch(messages)"},{"line_number":171,"context_line":"        except Exception:"},{"line_number":172,"context_line":"            not_processed_messages \u003d set(messages)"},{"line_number":173,"context_line":"            LOG.exception(_LE(\u0027Exception during handling messages: %s\u0027),"},{"line_number":174,"context_line":"                          messages)"},{"line_number":175,"context_line":"        for m in messages:"},{"line_number":176,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_973a2935","line":173,"updated":"2016-04-18 09:00:07.000000000","message":"This changes the error log, this is not refactoring.","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"}],"oslo_messaging/rpc/server.py":[{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"1755f65b004914c0d118d65c42410b0366257d96","unresolved":false,"context_lines":[{"line_number":123,"context_line":"    def _process_incoming_message(self, message):"},{"line_number":124,"context_line":"        try:"},{"line_number":125,"context_line":"            message.acknowledge()"},{"line_number":126,"context_line":"        except Exception:"},{"line_number":127,"context_line":"            LOG.exception(\"Can not acknowledge message: %s. Skip processing\","},{"line_number":128,"context_line":"                          message)"},{"line_number":129,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_b7e765e3","line":126,"updated":"2016-04-18 09:00:07.000000000","message":"This is new exception catching, not refactoring.","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"}],"oslo_messaging/server.py":[{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"1755f65b004914c0d118d65c42410b0366257d96","unresolved":false,"context_lines":[{"line_number":298,"context_line":""},{"line_number":299,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":300,"context_line":"class MessageHandlingServer(service.ServiceBase, _OrderedTaskRunner,"},{"line_number":301,"context_line":"                            driver_base.MessageHandler):"},{"line_number":302,"context_line":"    \"\"\"Server for handling messages."},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    Connect a transport to a dispatcher that knows how to process the"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_178e1978","line":301,"updated":"2016-04-18 09:00:07.000000000","message":"That doesn\u0027t looks a good layer separation to inherit something from another namespace.","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"1755f65b004914c0d118d65c42410b0366257d96","unresolved":false,"context_lines":[{"line_number":305,"context_line":"    message using an executor that knows how the app wants to create"},{"line_number":306,"context_line":"    new tasks."},{"line_number":307,"context_line":"    At least one method _process_incoming_message or"},{"line_number":308,"context_line":"    _process_incoming_message_list should be overridden by subclasses"},{"line_number":309,"context_line":"    \"\"\""},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"    def __init__(self, transport, dispatcher, executor\u003d\u0027blocking\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_9c17ee50","line":308,"updated":"2016-04-18 09:00:07.000000000","message":"This kind of interface doesn\u0027t looks sane and clear.","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"1755f65b004914c0d118d65c42410b0366257d96","unresolved":false,"context_lines":[{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        :param message: incoming message."},{"line_number":356,"context_line":"        \"\"\""},{"line_number":357,"context_line":"        self._work_executor.submit(self._process_incoming_message, message)"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    def on_incoming_message_list(self, messages):"},{"line_number":360,"context_line":"        \"\"\"Handles on_incoming_message_list event"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_5c4f1692","line":357,"updated":"2016-04-18 09:00:07.000000000","message":"We can directly use _process_incoming_message_list here","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"1755f65b004914c0d118d65c42410b0366257d96","unresolved":false,"context_lines":[{"line_number":384,"context_line":"        \"\"\""},{"line_number":385,"context_line":"        for message in messages:"},{"line_number":386,"context_line":"            try:"},{"line_number":387,"context_line":"                self._process_incoming_message(message)"},{"line_number":388,"context_line":"            except Exception:"},{"line_number":389,"context_line":"                LOG.exception("},{"line_number":390,"context_line":"                    \"Unexpected exception during handling message: %s\","}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_dc2206b9","line":387,"updated":"2016-04-18 09:00:07.000000000","message":"We call _process_incoming_message that will call back process_incoming_message_list, that looks weird.","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"0fbdaa9dd0078cc10f8b784ddbd902bc027ce564","unresolved":false,"context_lines":[{"line_number":384,"context_line":"        \"\"\""},{"line_number":385,"context_line":"        for message in messages:"},{"line_number":386,"context_line":"            try:"},{"line_number":387,"context_line":"                self._process_incoming_message(message)"},{"line_number":388,"context_line":"            except Exception:"},{"line_number":389,"context_line":"                LOG.exception("},{"line_number":390,"context_line":"                    \"Unexpected exception during handling message: %s\","}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_c6c03e7f","line":387,"in_reply_to":"1a122d0e_be59d400","updated":"2016-04-19 08:18:37.000000000","message":"I prefer to always handle list of messages, one message is just a particular case of the list. And having only one interface for the server is more readable.","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"},{"author":{"_account_id":8601,"name":"Dmitriy Ukhlov","email":"dukhlov@mirantis.com","username":"dukhlov"},"change_message_id":"58b17d914282f91e6ce652068b534b4137aeb587","unresolved":false,"context_lines":[{"line_number":384,"context_line":"        \"\"\""},{"line_number":385,"context_line":"        for message in messages:"},{"line_number":386,"context_line":"            try:"},{"line_number":387,"context_line":"                self._process_incoming_message(message)"},{"line_number":388,"context_line":"            except Exception:"},{"line_number":389,"context_line":"                LOG.exception("},{"line_number":390,"context_line":"                    \"Unexpected exception during handling message: %s\","}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_be59d400","line":387,"in_reply_to":"1a122d0e_dc2206b9","updated":"2016-04-18 23:14:07.000000000","message":"Driver\u0027s listener can call on_incoming_message_list or on_incoming_message depending on listener\u0027s implementation.\nOn the other side some servers want to work with single message, some servers are able to process message batch at once. So each server must override two methods: _process_incoming_message and _process_incoming_message_list to support possible listener implementations.\n\nBut it is extra work, therefore I added such default method implementation. It assumes that subclass will override only one of these two methods. If it is _process_incoming_message and listener calls _process_incoming_message_list - _process_incoming_message will be executed for all messages. If it is _process_incoming_message_list and listener calls _process_incoming_message - message will be wrapped in list and passed to _process_incoming_message_list\n\nIt is described in comments. Only one not desired behavior - StackOverflow if any method is not overridden by subclass.\n\nAnyway I see 3 options:\n1) keep as is\n2) revert as was before, listeners returns only list of messages\n3) implement _process_incoming_message_list and _process_incoming_message in each subclass\n\nWhat do you prefer?","commit_id":"c7803f347903350ab5b63df972f0aa0f4c58780a"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"73c726ff2e6350530c6bcc05b99d007ebee2e5c1","unresolved":false,"context_lines":[{"line_number":298,"context_line":""},{"line_number":299,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":300,"context_line":"class MessageHandlingServer(service.ServiceBase, _OrderedTaskRunner,"},{"line_number":301,"context_line":"                            driver_base.MessageHandler):"},{"line_number":302,"context_line":"    \"\"\"Server for handling messages."},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    Connect a transport to a dispatcher that knows how to process the"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_80d58eba","line":301,"updated":"2016-04-19 08:18:51.000000000","message":"I still think that MessageHandler class should be in this file.","commit_id":"6802856d01f02e09000b1e74730cbdfd899b327b"},{"author":{"_account_id":20864,"name":"Gevorg Davoian","email":"gdavoian@gmail.com","username":"gdavoian"},"change_message_id":"39048bafe11d65c3dc289db195fb8417180c9431","unresolved":false,"context_lines":[{"line_number":308,"context_line":"    def __init__(self, transport, dispatcher, executor\u003d\u0027blocking\u0027):"},{"line_number":309,"context_line":"        \"\"\"Construct a message handling server."},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        The dispatcher parameter is a callable which is invoked with context"},{"line_number":312,"context_line":"        and message dictionaries each time a message is received."},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        The executor parameter controls how incoming messages will be received"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_ed94e48b","line":311,"range":{"start_line":311,"start_character":38,"end_line":311,"end_character":46},"updated":"2016-04-20 14:23:40.000000000","message":"The dispatcher is not callable anymore.","commit_id":"c9b147dbe3cb819efb585ab9b173ce81e9624e99"},{"author":{"_account_id":8601,"name":"Dmitriy Ukhlov","email":"dukhlov@mirantis.com","username":"dukhlov"},"change_message_id":"d437d82ec3c3b2d1ba80ca5e44be8ee1d388720c","unresolved":false,"context_lines":[{"line_number":308,"context_line":"    def __init__(self, transport, dispatcher, executor\u003d\u0027blocking\u0027):"},{"line_number":309,"context_line":"        \"\"\"Construct a message handling server."},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        The dispatcher parameter is a callable which is invoked with context"},{"line_number":312,"context_line":"        and message dictionaries each time a message is received."},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        The executor parameter controls how incoming messages will be received"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_4b4dee98","line":311,"range":{"start_line":311,"start_character":38,"end_line":311,"end_character":46},"in_reply_to":"1a122d0e_ed94e48b","updated":"2016-04-20 20:48:06.000000000","message":"Done","commit_id":"c9b147dbe3cb819efb585ab9b173ce81e9624e99"}]}