)]}'
{"oslo/messaging/_drivers/amqpdriver.py":[{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"74953f3841f89680d7d28baca14e025804deeb3c","unresolved":false,"context_lines":[{"line_number":22,"context_line":"import uuid"},{"line_number":23,"context_line":"import weakref"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"import futures"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from oslo import messaging"},{"line_number":28,"context_line":"from oslo.messaging._drivers import amqp as rpc_amqp"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_9376b9d3","line":25,"updated":"2014-08-12 02:03:04.000000000","message":"Pretty sure u want this instead.\n\n  from concurrent import futures\n\nSince futures is in python3 this will correctly find it there as well as correctly in python2 when the futures library is used.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"74953f3841f89680d7d28baca14e025804deeb3c","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_d34bd183","line":99,"updated":"2014-08-12 02:03:04.000000000","message":"Any reason for this besides just helping the GC?","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"69dabc191ca9d814e47bb9630a4546191dad6a74","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_e242e53c","line":99,"in_reply_to":"1abeadc6_13257608","updated":"2014-08-21 18:01:58.000000000","message":"That\u0027s overcomplexification. These messages are stored in self only until they\u0027re popped from it by poll(). This ref cycle exists for some short amount of time and while it exists, both Listener and Message are actually needed. If anyone else stores Message, we won\u0027t have ref cycle after poll().","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"79111ea166ff8f5b3c7973b4c559186106963b34","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_781922d6","line":99,"in_reply_to":"1abeadc6_38bfaaab","updated":"2014-08-21 19:37:21.000000000","message":"Ok, you convinced me here. :)","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"35c5f05a4dda9e84ebd154454f4a6a35942bd875","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_13257608","line":99,"in_reply_to":"1abeadc6_4872dd11","updated":"2014-08-21 17:17:58.000000000","message":"From reading some python docs: \"Because reference cycles are take computational work to discover, garbage collection must be a scheduled activity. Python schedules garbage collection based upon a threshold of object allocations and object deallocations. When the number of allocations minus the number of deallocations are greater than the threshold number, the garbage collector is run.\". \n\nBasically it means that there is no guarantee that GC will ever run (or run in reasonable time). That\u0027s why I think that it\u0027s better not to have reference cycles than to wait for GC to collect them.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"40ceb8138bc04071f1b3a83132602ad9076de6f4","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_ed69fbbe","line":99,"in_reply_to":"1abeadc6_a2c1d255","updated":"2014-08-21 16:08:02.000000000","message":"weakref overhead seem negligible to me compared with opening new channels and other unnecessary stuff that we do. My main reasoning for using weakrefs is to remove cyclic references between objects. I would like to have a directed graph of relations with a well know root object that would be easily created and destroyed. Weakref here is used cause dispatcher implementation may hold a reference for a message object for some reason and it would prevent listener object to be garbage collected.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"1ac996b24fc9d5ba5789e91091a9754bae882968","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_e53bcfce","line":99,"in_reply_to":"1abeadc6_e242e53c","updated":"2014-08-21 18:15:46.000000000","message":"\u003e If anyone else stores Message, we won\u0027t have ref cycle after poll().\n\nWe won\u0027t have ref cycle however this message object will prevent listener object from being garbage collected, no? And not collecting a Listener will prevent associated connection from closing.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"e30d5e972057549bba45851ebe87b809cfa2652d","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_38bfaaab","line":99,"in_reply_to":"1abeadc6_e53bcfce","updated":"2014-08-21 19:02:13.000000000","message":"Very good case you\u0027re presenting - Listener can get closed while messages are active. The only place where message is being held outside listener is in executor\u0027s loop for the time this message is being handled. What happens when connection is closed, but message isn\u0027t handled yet? Someone might call reply() and... Get ReferenceError! By using weakref.proxy here you basically allow Message instances to fall in inconsistent state and that\u0027s bad.\n\nAnd another point. This change provides a solution without a problem: references to messages are never held for a long time outside Listener anyway.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f335f4995b6aa58efc37a2a382621cb068c1ce0c","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_4872dd11","line":99,"in_reply_to":"1abeadc6_ed69fbbe","updated":"2014-08-21 16:58:06.000000000","message":"But once your dispatcher gets collected, these cyclic references will be properly handled by next GC run. That\u0027s why GC is there in the first place. I don\u0027t see why we should add (even negligible) overhead and complication here.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"a5a408a085b5a9bacbdf19690f4ac745c0801810","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_afd334d1","line":99,"in_reply_to":"baada198_47437196","updated":"2014-08-12 17:15:56.000000000","message":"Ah, ok, maybe worth a inline comment then that explains why?","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f498ed3814f7a96b3c96c56856d490b6a83ec7af","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_a2c1d255","line":99,"in_reply_to":"baada198_47437196","updated":"2014-08-21 15:43:58.000000000","message":"Waiter waits for Driver\u0027s death to stop its thread. Do we still need to add weakref overhead to production code here?\n\nIf it\u0027s still needed we can patch AMQPIncomingMessage in tests to create this proxy.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"b02741a6802c0007abcdaea131061864d17657ba","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        unique_id \u003d self.msg_id_cache.check_duplicate_message(message)"},{"line_number":97,"context_line":"        ctxt \u003d rpc_amqp.unpack_context(self.conf, message)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.incoming.append(AMQPIncomingMessage(weakref.proxy(self),"},{"line_number":100,"context_line":"                                                 ctxt.to_dict(),"},{"line_number":101,"context_line":"                                                 message,"},{"line_number":102,"context_line":"                                                 unique_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_47437196","line":99,"in_reply_to":"baada198_d34bd183","updated":"2014-08-12 10:22:01.000000000","message":"This is needed mostly for tests. Since we start a new thread for ReplyWaiter we need to correctly stop and destroy it once test is finished. This is done only if objects are garbage collected immediately when test stops (e.g. by reference counted gc). Otherwise tests was handing for me cause they spawned hundreds of threads that would stay in background","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f335f4995b6aa58efc37a2a382621cb068c1ce0c","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_33e51238","line":133,"range":{"start_line":133,"start_character":55,"end_line":133,"end_character":59},"updated":"2014-08-21 16:58:06.000000000","message":"Overall I see that there are too many weakrefs in this code because you keep reference to self here. You shouldn\u0027t do that. You should just decouple self from thread that\u0027s running instead. It needs only a flag and a connection object after all. Btw, doing that you can avoid subclassing Thread (which is a bad taste).","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f335f4995b6aa58efc37a2a382621cb068c1ce0c","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_689799ec","line":133,"updated":"2014-08-21 16:58:06.000000000","message":"self._driver is not used anywhere. No need to create a proxy here.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"880625540ea1cb5188602a3bd6ddb498dd885454","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_b65bcbf4","line":133,"in_reply_to":"1abeadc6_1bef48ea","updated":"2014-08-21 20:08:12.000000000","message":"1. If that\u0027s really the case, the simpler way would be to explicitly stop extra threads in tests (which in term might break that ref loop).\n\n2. You can pass _pending_replies dict to that thread in constructor or call arguments.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"e30d5e972057549bba45851ebe87b809cfa2652d","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_78892274","line":133,"in_reply_to":"1abeadc6_251b0774","updated":"2014-08-21 19:02:13.000000000","message":"Of course not. You don\u0027t need to care who reference your object. You need to care what objects you\u0027re referencing to.\n\nBtw, right here you\u0027re keeping a referenc to self in run() loop. If you separate thread handling you won\u0027t have this loop and I bet all those problems will go away.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"35c5f05a4dda9e84ebd154454f4a6a35942bd875","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_33fa7243","line":133,"in_reply_to":"1abeadc6_33e51238","updated":"2014-08-21 17:17:58.000000000","message":"Not sure I understand. How decoupling Waiter from thread will help in stopping waiter thread? And why subclassing a thread is a bad idea? I need to have a class with a run method and this method should run in a separate thread. Why shouldn\u0027t I subclass a thread in such case?","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"69dabc191ca9d814e47bb9630a4546191dad6a74","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_a2fc6d73","line":133,"in_reply_to":"1abeadc6_33fa7243","updated":"2014-08-21 18:01:58.000000000","message":"Right here you create a weak reference to Driver that you don\u0027t need. You don\u0027t have any references to Waiter but the one in Driver. So all you need is to have a separate object (thread) that would be able to stop when Waiter gets collected.\n\nWhich brings me to the second point. You don\u0027t extend Thread functionality here, you just use it. It\u0027s like subclassing int when you need a counter. Btw, currently Waiter object are stored in Python internals (threading module) - because it\u0027s a Thread. You don\u0027t want this, don\u0027t you?\n\nSo you can create a StoppableThread class that would provide a way to stop thread (if you like subclassing Thread) or incapsulate all data necessary for that thread in a separate object (WaiterLoop or whatever). In any case you\u0027ll be able to have one object that handles the thread (loop) and another object that provides Waiter interface to it. And you can easily kill thread (loop) in the first object when the second one dies. One tight couple of objects allows you to never care about them all over the place (as it happens now with weakrefs).","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"cab0fc7801ff2e0e09aa3239125f78c6d3e8ae87","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_71069d1e","line":133,"in_reply_to":"1abeadc6_36edfbb7","updated":"2014-08-21 20:29:17.000000000","message":"1. We just were talking about tests. If that\u0027s also the case for SIGHUP handling, then it\u0027s probably bad, but we should verify that.\n\n2. Right now you have ReplyWaiter bolted to WorkerThread and they do communicate through shared dict. Not much changes in this respect. To keep it clear from semantic pov you can keep all necessary method(s) (or class) in Waiter class.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f144cb3263d8ed7f4f4e12484707d57e64eeb0ed","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_be2e30a3","line":133,"in_reply_to":"1abeadc6_51bc41dc","updated":"2014-08-22 09:44:30.000000000","message":"1. From the first glance on the bug and the fix, it looks like it goes in a wrong direction. Breaking ref loops with weakrefs instead of fixing relationships between objects; relying on GC to implicitly close all our connections instead of explicitly closing them at the point we know they should be closed (SIGHUP). All so unpythonic.\n\n2. Smth like this (not tested): http://paste.openstack.org/show/98629/","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"efe7f3042ddf022cd39f9e9b376794ce0cfabe55","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_51bc41dc","line":133,"in_reply_to":"1abeadc6_71069d1e","updated":"2014-08-21 20:39:33.000000000","message":"1. there is a bug about it - https://bugs.launchpad.net/oslo.messaging/+bug/1335086\n\n2. Could you write a small prototype explaining your implementation proposal? Cause I cannot understand how it should look like.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"79111ea166ff8f5b3c7973b4c559186106963b34","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_1bef48ea","line":133,"in_reply_to":"1abeadc6_78892274","updated":"2014-08-21 19:37:21.000000000","message":"\u003eOf course not. You don\u0027t need to care who reference your object. You need to care what objects you\u0027re referencing to.\n\nI need such strict rules to makes sure that objects are destroyed correctly in tests (Otherwise they spawn too many threads).\n\n\u003eBtw, right here you\u0027re keeping a referenc to self in run() loop. If you separate thread handling you won\u0027t have this loop and I bet all those problems will go away.\n\nHow do we pass results to waiting futures if we don\u0027t have references to ReplyWaiter from worker thread?","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"1ac996b24fc9d5ba5789e91091a9754bae882968","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_251b0774","line":133,"in_reply_to":"1abeadc6_a2fc6d73","updated":"2014-08-21 18:15:46.000000000","message":"\u003eOne tight couple of objects allows you to never care about them all over the place (as it happens now with weakrefs).\n\nI will still need to maintain exactly 1 strong ref to ReplyWaiter object (owned by driver). Otherwise it would not be destroyed when driver gets destroyed.\n\nSo I agree that solution that you propose may be better but I don\u0027t think that it solves *all* problems with correct Waiter destruction.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"04ecb0fa35050e65a095c9111f00b5baa46b9d02","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.reply_q \u003d \u0027reply_\u0027 + uuid.uuid4().hex"},{"line_number":131,"context_line":"        self._connection \u003d driver._get_connection(pooled\u003dFalse)"},{"line_number":132,"context_line":"        self.allowed_remote_exmods \u003d driver._allowed_remote_exmods"},{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_36edfbb7","line":133,"in_reply_to":"1abeadc6_b65bcbf4","updated":"2014-08-21 20:16:25.000000000","message":"1. I don\u0027t think that need to explicitly stop a thread is a good thing. It breaks incapsulation of ReplyWaiter implementation. In addition some applications use SIGHUP to reload config and recreate objects. It\u0027s much simpler to create a new driver object and rely on GC to close everything than require application to explicitly cleanup driver.\n\n2. IMHO the result will be much more complicated and error prone that what I\u0027m proposing. Right now we have 2 objects - ReplyWaiter that stops itself when it\u0027s owner (Driver) is destroyed. If we do as you propose we\u0027ll have 3 objects (Driver + ReplyWaiter + WorkerThread) and in addition waiter and worker thread are going to communicate through shared dict (sic!)","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f498ed3814f7a96b3c96c56856d490b6a83ec7af","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"        self._pending_replies \u003d {}"},{"line_number":139,"context_line":"        self._running \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_1fc9fd58","line":136,"range":{"start_line":136,"start_character":49,"end_line":136,"end_character":68},"updated":"2014-08-21 15:43:58.000000000","message":"Why use weakref here?","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"40ceb8138bc04071f1b3a83132602ad9076de6f4","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"        self._pending_replies \u003d {}"},{"line_number":139,"context_line":"        self._running \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_ad7aa374","line":136,"in_reply_to":"1abeadc6_1fc9fd58","updated":"2014-08-21 16:08:02.000000000","message":"Reason is the same as above: \navoid cyclic reference between object and keep graph nice and clean.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f335f4995b6aa58efc37a2a382621cb068c1ce0c","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        self._driver \u003d weakref.proxy(driver, lambda x: self.stop())"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self._connection.declare_direct_consumer(self.reply_q,"},{"line_number":136,"context_line":"                                                 weakref.proxy(self))"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"        self._pending_replies \u003d {}"},{"line_number":139,"context_line":"        self._running \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_886a35ec","line":136,"in_reply_to":"1abeadc6_ad7aa374","updated":"2014-08-21 16:58:06.000000000","message":"And for the same reason as above you shouldn\u0027t complicate your code with this.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f498ed3814f7a96b3c96c56856d490b6a83ec7af","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def run(self):"},{"line_number":143,"context_line":"        self._running \u003d True"},{"line_number":144,"context_line":"        while self._running:"},{"line_number":145,"context_line":"            try:"},{"line_number":146,"context_line":"                self._connection.consume(timeout\u003d1)"},{"line_number":147,"context_line":"            except rpc_common.Timeout:"},{"line_number":148,"context_line":"                # NOTE(Alexei_987) that\u0027s normal we only need timeout"},{"line_number":149,"context_line":"                # to check for stop signal and exit thread normally"},{"line_number":150,"context_line":"                pass"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def stop(self):"},{"line_number":153,"context_line":"        self._running \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_3f9dd966","line":150,"range":{"start_line":144,"start_character":8,"end_line":150,"end_character":20},"updated":"2014-08-21 15:43:58.000000000","message":"We can avoid busy wait here when there\u0027s no requests in progress. Smth like this:\n\n    while self._running:\n        if not self._pending_replies:\n            self._calls_event.wait()\n            self._calls_event.clear()\n        while self._running and self._pending_replies:\n            \u003csame loop contents\u003e\n\nself._calls_event would be a threading.Event that\u0027s set in stop() and listen() (after future is added to the dict)","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"40ceb8138bc04071f1b3a83132602ad9076de6f4","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            except rpc_common.Timeout:"},{"line_number":148,"context_line":"                # NOTE(Alexei_987) that\u0027s normal we only need timeout"},{"line_number":149,"context_line":"                # to check for stop signal and exit thread normally"},{"line_number":150,"context_line":"                pass"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def stop(self):"},{"line_number":153,"context_line":"        self._running \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_4d8bcf67","line":150,"in_reply_to":"1abeadc6_3f9dd966","updated":"2014-08-21 16:08:02.000000000","message":"Great advice... will do.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f498ed3814f7a96b3c96c56856d490b6a83ec7af","unresolved":false,"context_lines":[{"line_number":155,"context_line":"    @contextlib.contextmanager"},{"line_number":156,"context_line":"    def listen(self, msg_id):"},{"line_number":157,"context_line":"        call \u003d futures.Future()"},{"line_number":158,"context_line":"        self._pending_replies[msg_id] \u003d call"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        try:"},{"line_number":161,"context_line":"            yield call"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_df9ce55f","line":158,"updated":"2014-08-21 15:43:58.000000000","message":"We might want to add a guard here to prevent ugly problems in case smth wrong with msg_id generation (e.g. in tests):\n\n    assert msg_id not in self._pending_replies","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"40ceb8138bc04071f1b3a83132602ad9076de6f4","unresolved":false,"context_lines":[{"line_number":155,"context_line":"    @contextlib.contextmanager"},{"line_number":156,"context_line":"    def listen(self, msg_id):"},{"line_number":157,"context_line":"        call \u003d futures.Future()"},{"line_number":158,"context_line":"        self._pending_replies[msg_id] \u003d call"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        try:"},{"line_number":161,"context_line":"            yield call"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_adf1c302","line":158,"in_reply_to":"1abeadc6_df9ce55f","updated":"2014-08-21 16:08:02.000000000","message":"makes sense","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"74953f3841f89680d7d28baca14e025804deeb3c","unresolved":false,"context_lines":[{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        call \u003d self._pending_replies.get(msg_id)"},{"line_number":175,"context_line":"        if not call:"},{"line_number":176,"context_line":"            LOG.warn(\u0027Message with no call associated with it - %s\u0027 % message)"},{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        if message[\u0027failure\u0027]:"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_ee68eebd","line":176,"updated":"2014-08-12 02:03:04.000000000","message":"How about better named, \u0027Message with no future associated with it\u0027 since its really a future object, not a call object.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"b7d25abf672d0aab87c17bd41471c515a6fb7e20","unresolved":false,"context_lines":[{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        call \u003d self._pending_replies.get(msg_id)"},{"line_number":175,"context_line":"        if not call:"},{"line_number":176,"context_line":"            LOG.warn(\u0027Message with no call associated with it - %s\u0027 % message)"},{"line_number":177,"context_line":"            return"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        if message[\u0027failure\u0027]:"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_4e663a56","line":176,"in_reply_to":"baada198_ee68eebd","updated":"2014-08-12 03:00:40.000000000","message":"Also this can happen if the future expired/timeout happened, so the message might want to reflect that also?","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"74953f3841f89680d7d28baca14e025804deeb3c","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        message.update({\u0027_msg_id\u0027: msg_id,"},{"line_number":232,"context_line":"                        \u0027_reply_q\u0027: waiter.reply_q})"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        with waiter.listen(msg_id) as call:"},{"line_number":235,"context_line":"            self._send(target, message, timeout, retry)"},{"line_number":236,"context_line":"            try:"},{"line_number":237,"context_line":"                return call.result(timeout)"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_2e94a68a","line":234,"updated":"2014-08-12 02:03:04.000000000","message":"I think u want to call the following also before this `_send` occurs??\n\nhttps://docs.python.org/dev/library/concurrent.futures.html#concurrent.futures.Future.set_running_or_notify_cancel","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"66dfd7ef1fba29155918525226354aa9398192ff","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        message.update({\u0027_msg_id\u0027: msg_id,"},{"line_number":232,"context_line":"                        \u0027_reply_q\u0027: waiter.reply_q})"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        with waiter.listen(msg_id) as call:"},{"line_number":235,"context_line":"            self._send(target, message, timeout, retry)"},{"line_number":236,"context_line":"            try:"},{"line_number":237,"context_line":"                return call.result(timeout)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_148f0d2b","line":234,"in_reply_to":"1abeadc6_df01456b","updated":"2014-08-22 04:27:43.000000000","message":"Fair enough ;)","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f498ed3814f7a96b3c96c56856d490b6a83ec7af","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        message.update({\u0027_msg_id\u0027: msg_id,"},{"line_number":232,"context_line":"                        \u0027_reply_q\u0027: waiter.reply_q})"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        with waiter.listen(msg_id) as call:"},{"line_number":235,"context_line":"            self._send(target, message, timeout, retry)"},{"line_number":236,"context_line":"            try:"},{"line_number":237,"context_line":"                return call.result(timeout)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_df01456b","line":234,"in_reply_to":"baada198_2e94a68a","updated":"2014-08-21 15:43:58.000000000","message":"We don\u0027t have any cancellation here, so why bother?","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"74953f3841f89680d7d28baca14e025804deeb3c","unresolved":false,"context_lines":[{"line_number":232,"context_line":"                        \u0027_reply_q\u0027: waiter.reply_q})"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        with waiter.listen(msg_id) as call:"},{"line_number":235,"context_line":"            self._send(target, message, timeout, retry)"},{"line_number":236,"context_line":"            try:"},{"line_number":237,"context_line":"                return call.result(timeout)"},{"line_number":238,"context_line":"            except futures.TimeoutError:"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_4e3abac4","line":235,"updated":"2014-08-12 02:03:04.000000000","message":"Should the sending time be deducted from the waiting for result time?\n\nCan sending take X seconds, with a total timeout of Y, and then the result waiting time should really be Y - X?","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f498ed3814f7a96b3c96c56856d490b6a83ec7af","unresolved":false,"context_lines":[{"line_number":247,"context_line":"            else:"},{"line_number":248,"context_line":"                topic \u003d target.topic"},{"line_number":249,"context_line":"                if target.server:"},{"line_number":250,"context_line":"                    topic \u003d \u0027%s.%s\u0027 % (target.topic, target.server)"},{"line_number":251,"context_line":"                conn.topic_send(exchange_name\u003dself._get_exchange(target),"},{"line_number":252,"context_line":"                                topic\u003dtopic, msg\u003dmessage, timeout\u003dtimeout,"},{"line_number":253,"context_line":"                                retry\u003dretry)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_1f881dc4","line":250,"range":{"start_line":250,"start_character":39,"end_line":250,"end_character":51},"updated":"2014-08-21 15:43:58.000000000","message":"Replace with \"topic\" (or move previous topic var assignment to else: branch)","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f498ed3814f7a96b3c96c56856d490b6a83ec7af","unresolved":false,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    def send_notification(self, target, ctxt, message, version, retry\u003dNone):"},{"line_number":256,"context_line":"        self._prepare_message(message, ctxt)"},{"line_number":257,"context_line":"        if version \u003d\u003d 2.0:"},{"line_number":258,"context_line":"            message \u003d rpc_common.serialize_msg(message)"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"        with self._get_connection() as conn:"},{"line_number":261,"context_line":"            conn.notify_send(self._get_exchange(target),"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_62b97a14","line":258,"range":{"start_line":257,"start_character":8,"end_line":258,"end_character":55},"updated":"2014-08-21 15:43:58.000000000","message":"Ugly hardcode. (legacy, I know)","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"74953f3841f89680d7d28baca14e025804deeb3c","unresolved":false,"context_lines":[{"line_number":261,"context_line":"            conn.notify_send(self._get_exchange(target),"},{"line_number":262,"context_line":"                             target.topic, message, retry\u003dretry)"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    # FIXME(markmc): remove this temporary hack"},{"line_number":265,"context_line":"    class Context(object):"},{"line_number":266,"context_line":"        def __init__(self, d):"},{"line_number":267,"context_line":"            self.d \u003d d"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_6e177e46","line":264,"updated":"2014-08-12 02:03:04.000000000","message":"What the heck, lol.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f498ed3814f7a96b3c96c56856d490b6a83ec7af","unresolved":false,"context_lines":[{"line_number":261,"context_line":"            conn.notify_send(self._get_exchange(target),"},{"line_number":262,"context_line":"                             target.topic, message, retry\u003dretry)"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    # FIXME(markmc): remove this temporary hack"},{"line_number":265,"context_line":"    class Context(object):"},{"line_number":266,"context_line":"        def __init__(self, d):"},{"line_number":267,"context_line":"            self.d \u003d d"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_226782ea","line":264,"in_reply_to":"baada198_47f1b162","updated":"2014-08-21 15:43:58.000000000","message":"You can just remove it and pass ctxt to rpc_amqp.pack_context as is. (see pack_context implementation)","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"b02741a6802c0007abcdaea131061864d17657ba","unresolved":false,"context_lines":[{"line_number":261,"context_line":"            conn.notify_send(self._get_exchange(target),"},{"line_number":262,"context_line":"                             target.topic, message, retry\u003dretry)"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    # FIXME(markmc): remove this temporary hack"},{"line_number":265,"context_line":"    class Context(object):"},{"line_number":266,"context_line":"        def __init__(self, d):"},{"line_number":267,"context_line":"            self.d \u003d d"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_47f1b162","line":264,"in_reply_to":"baada198_6e177e46","updated":"2014-08-12 10:22:01.000000000","message":"I\u0027m not sure why this is needed. Some kind of legacy hack :)","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f498ed3814f7a96b3c96c56856d490b6a83ec7af","unresolved":false,"context_lines":[{"line_number":270,"context_line":"            return self.d"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"    @classmethod"},{"line_number":273,"context_line":"    def _prepare_message(cls, message, ctxt):"},{"line_number":274,"context_line":"        context \u003d cls.Context(ctxt)"},{"line_number":275,"context_line":"        rpc_amqp._add_unique_id(message)"},{"line_number":276,"context_line":"        rpc_amqp.pack_context(message, context)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_dfd4e5be","line":273,"updated":"2014-08-21 15:43:58.000000000","message":"Can we make this return new version of message? Changing it in-place seems very bad.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"f498ed3814f7a96b3c96c56856d490b6a83ec7af","unresolved":false,"context_lines":[{"line_number":272,"context_line":"    @classmethod"},{"line_number":273,"context_line":"    def _prepare_message(cls, message, ctxt):"},{"line_number":274,"context_line":"        context \u003d cls.Context(ctxt)"},{"line_number":275,"context_line":"        rpc_amqp._add_unique_id(message)"},{"line_number":276,"context_line":"        rpc_amqp.pack_context(message, context)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"    def listen(self, target):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_82ebf611","line":275,"range":{"start_line":275,"start_character":8,"end_line":275,"end_character":40},"updated":"2014-08-21 15:43:58.000000000","message":"How does this unique_id differ from our msg_id? Can we use it instead of generating new uuid?","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"},{"author":{"_account_id":7763,"name":"Alexei Kornienko","email":"akornienko@mirantis.com","username":"akornienko"},"change_message_id":"40ceb8138bc04071f1b3a83132602ad9076de6f4","unresolved":false,"context_lines":[{"line_number":272,"context_line":"    @classmethod"},{"line_number":273,"context_line":"    def _prepare_message(cls, message, ctxt):"},{"line_number":274,"context_line":"        context \u003d cls.Context(ctxt)"},{"line_number":275,"context_line":"        rpc_amqp._add_unique_id(message)"},{"line_number":276,"context_line":"        rpc_amqp.pack_context(message, context)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"    def listen(self, target):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_4d674fb7","line":275,"in_reply_to":"1abeadc6_82ebf611","updated":"2014-08-21 16:08:02.000000000","message":"From what I know unique_id prevents process from receiving same message twice (yeah I know it\u0027s ugly). It is also present for cast messages (which doesn\u0027t contain msg_id). We can assign msg_id\u003dunique_id if you think that it\u0027s worth it.","commit_id":"fef064be4c2956a4ea197d95ba1602a8aae62165"}]}