)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"dae3ddc922f0ece9d2a723cbec7e503e748b8bd4","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add a no-op wait method to NetworkInfo"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When a virtual machine deploy fails first time network_info"},{"line_number":10,"context_line":"object returns \u0027NetworkInfoAsyncWrapper\u0027 which has the wait"},{"line_number":11,"context_line":"attribute but subsequent deploy failures return \u0027NetworkInfo\u0027"},{"line_number":12,"context_line":"object which does not have wait attribute thus throwing an"},{"line_number":13,"context_line":"error."},{"line_number":14,"context_line":"Here a wait no-op is added to NetworkInfo object to prevent"},{"line_number":15,"context_line":"such errors."},{"line_number":16,"context_line":"Change-Id: Id7a71b2eb46ea7df19e7da0afbc0eafa87cac965"},{"line_number":17,"context_line":"Closes-Bug: 1636109"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba5da102_333fffd7","line":15,"range":{"start_line":9,"start_character":0,"end_line":15,"end_character":12},"updated":"2016-11-04 12:28:36.000000000","message":"I think this can be clearer. E.g.:\n\nThe normal deploy flow uses a NetworkInfoAsyncWrapper for network allocation, and because of that it makes calls to that class\u0027s wait method in places to make sure it has completed. During a reschedule where the network was allocated by a previous build attempt, a NetworkInfo instance is retrieved instead, which does not have a wait method. This then results in an exception complaining about \u0027NetworkInfo\u0027 object has no attribute \u0027wait\u0027. to fix this, adding a stub wait method to NetworkInfo. Alternatively could have used isinstance to avoid making wait calls on NetworkInfo, but we generally try to avoid isinstance.\n\nThis fixes a regression issue caused by https://github.com/openstack/nova/commit/61fc1b9ee11e416aecbf3a29e1d150a53fc890e8 , which reverted the previous fix made under https://github.com/openstack/nova/commit/24a04c405ab2c98e52ea1edf8775489907526c6d","commit_id":"074eb82d79e064b234aef07db0a644898d90283f"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"dae3ddc922f0ece9d2a723cbec7e503e748b8bd4","unresolved":false,"context_lines":[{"line_number":13,"context_line":"error."},{"line_number":14,"context_line":"Here a wait no-op is added to NetworkInfo object to prevent"},{"line_number":15,"context_line":"such errors."},{"line_number":16,"context_line":"Change-Id: Id7a71b2eb46ea7df19e7da0afbc0eafa87cac965"},{"line_number":17,"context_line":"Closes-Bug: 1636109"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba5da102_33ba9f6f","line":16,"updated":"2016-11-04 12:28:36.000000000","message":"add a blank line to separate from the text above","commit_id":"074eb82d79e064b234aef07db0a644898d90283f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"045c02c5bb216ebe5c8008b603b2192c2ede4671","unresolved":false,"context_lines":[{"line_number":19,"context_line":"future and may not know to make the isinstance/hasattr check."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This fixes a regression issue caused by"},{"line_number":22,"context_line":"https://github.com/openstack/nova/commit/61fc1b9ee11e416aecbf3a29e1d150a53fc890e8 ,"},{"line_number":23,"context_line":"which reverted the previous fix made under"},{"line_number":24,"context_line":"https://github.com/openstack/nova/commit/24a04c405ab2c98e52ea1edf8775489907526c6d"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9a629dbe_d3ae47e5","line":22,"updated":"2016-11-09 18:33:24.000000000","message":"nit: this can just be 61fc1b9ee11e416aecbf3a29e1d150a53fc890e8, gerrit will link that properly.","commit_id":"706121156c699d2575f2345c2beba46c49e24a10"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"045c02c5bb216ebe5c8008b603b2192c2ede4671","unresolved":false,"context_lines":[{"line_number":21,"context_line":"This fixes a regression issue caused by"},{"line_number":22,"context_line":"https://github.com/openstack/nova/commit/61fc1b9ee11e416aecbf3a29e1d150a53fc890e8 ,"},{"line_number":23,"context_line":"which reverted the previous fix made under"},{"line_number":24,"context_line":"https://github.com/openstack/nova/commit/24a04c405ab2c98e52ea1edf8775489907526c6d"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Change-Id: Id7a71b2eb46ea7df19e7da0afbc0eafa87cac965"},{"line_number":27,"context_line":"Closes-Bug: 1636109"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9a629dbe_13b51f30","line":24,"updated":"2016-11-09 18:33:24.000000000","message":"ditto","commit_id":"706121156c699d2575f2345c2beba46c49e24a10"}],"nova/network/model.py":[{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"dae3ddc922f0ece9d2a723cbec7e503e748b8bd4","unresolved":false,"context_lines":[{"line_number":470,"context_line":"    def wait(self, do_raise\u003dTrue):"},{"line_number":471,"context_line":"        \"\"\"A no-op method."},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        This is useful to avoid type-checking when NetworkInfo might be"},{"line_number":474,"context_line":"        subclassed with NetworkInfoAsyncWrapper"},{"line_number":475,"context_line":"        \"\"\""},{"line_number":476,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_d377a306","line":473,"updated":"2016-11-04 12:28:36.000000000","message":"we need to make this clearer so that it isn\u0027t removed again.","commit_id":"074eb82d79e064b234aef07db0a644898d90283f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c07603f0752e0a45c412e95de126c5cc77897107","unresolved":false,"context_lines":[{"line_number":467,"context_line":"            network_info \u003d jsonutils.loads(network_info)"},{"line_number":468,"context_line":"        return cls([VIF.hydrate(vif) for vif in network_info])"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def wait(self, do_raise\u003dTrue):"},{"line_number":471,"context_line":"        \"\"\"A no-op method."},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        This is useful to avoid type-checking when NetworkInfo might be"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_5e4b8f2b","line":470,"updated":"2016-11-07 16:25:09.000000000","message":"In a point of view of OO that looks really strange... It\u0027s clearly a hack. We add a method here because a child. I don\u0027t think it\u0027s the good way to fix that, we should verify which object it is before to call wait that is because we know exactly what we do...","commit_id":"bf27b8d9984d9e3816c08d0443b5185e9960505d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"35f0cf8032751264027a13fa251b7a1a7c56c569","unresolved":false,"context_lines":[{"line_number":467,"context_line":"            network_info \u003d jsonutils.loads(network_info)"},{"line_number":468,"context_line":"        return cls([VIF.hydrate(vif) for vif in network_info])"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def wait(self, do_raise\u003dTrue):"},{"line_number":471,"context_line":"        \"\"\"A no-op method."},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        This is useful to avoid type-checking when NetworkInfo might be"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_5b24135f","line":470,"in_reply_to":"9a629dbe_0ad24b5c","updated":"2016-11-09 16:43:41.000000000","message":"I don\u0027t care about performance, I care about spaghetti code with isinstance or hasattr all over the place. Because invariably we\u0027ll add a new call to nw_info.wait() at some point that won\u0027t know it needs to do the isinstance or hasattr and it will fail at runtime because we\u0027ll have mocked out the wait() call in unit tests.\n\nAs noted, this is how it always worked before newton, so I\u0027m inclined to just add this back in here. If there are concerns about it being removed again later, than make the comments in this method more forceful about why this is needed. But the unit tests should also cover it from being accidentially removed.","commit_id":"bf27b8d9984d9e3816c08d0443b5185e9960505d"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"f8b81d98d3feab7537d043588848e73b1b794adb","unresolved":false,"context_lines":[{"line_number":467,"context_line":"            network_info \u003d jsonutils.loads(network_info)"},{"line_number":468,"context_line":"        return cls([VIF.hydrate(vif) for vif in network_info])"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def wait(self, do_raise\u003dTrue):"},{"line_number":471,"context_line":"        \"\"\"A no-op method."},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        This is useful to avoid type-checking when NetworkInfo might be"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_be7e6bac","line":470,"in_reply_to":"9a629dbe_5e4b8f2b","updated":"2016-11-07 16:32:54.000000000","message":"I agree that it\u0027s not really proper, but sometimes hacks are justified. There are performance implications with isinstance. When discussed on irc, this approach was preferred. It\u0027s also worth noting that this is the approach that was originally used years ago, and then mistakenly removed a few months ago causing this regression issue.","commit_id":"bf27b8d9984d9e3816c08d0443b5185e9960505d"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"95210a8b9ed394aa636d42e6a8a8591b61c02df5","unresolved":false,"context_lines":[{"line_number":467,"context_line":"            network_info \u003d jsonutils.loads(network_info)"},{"line_number":468,"context_line":"        return cls([VIF.hydrate(vif) for vif in network_info])"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def wait(self, do_raise\u003dTrue):"},{"line_number":471,"context_line":"        \"\"\"A no-op method."},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        This is useful to avoid type-checking when NetworkInfo might be"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_0ad24b5c","line":470,"in_reply_to":"9a629dbe_67a2224d","updated":"2016-11-09 15:41:41.000000000","message":"maybe. Though there\u0027s really nothing to the forward-compatibility argument. Any future classes would subclass NetworkInfo, so they would have the wait method by virtue of the current proposal.\n\nWe can do whatever the nova core team wants. Need them to weigh in.","commit_id":"bf27b8d9984d9e3816c08d0443b5185e9960505d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0aed3292e3e9a3725683d9f599b88fb22af1ee09","unresolved":false,"context_lines":[{"line_number":467,"context_line":"            network_info \u003d jsonutils.loads(network_info)"},{"line_number":468,"context_line":"        return cls([VIF.hydrate(vif) for vif in network_info])"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def wait(self, do_raise\u003dTrue):"},{"line_number":471,"context_line":"        \"\"\"A no-op method."},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        This is useful to avoid type-checking when NetworkInfo might be"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_67a2224d","line":470,"in_reply_to":"9a629dbe_99e16001","updated":"2016-11-09 15:34:35.000000000","message":"What about hasattr?  Or try/except AttributeError?  Those should have performance equivalent to the original method call - and they have the bonus effect of being more forward-compatible (in case we add a NetworkInfoSomeOtherKindOfWrapper in the future that also lacks a wait method).","commit_id":"bf27b8d9984d9e3816c08d0443b5185e9960505d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"82be367c8ee901c5f5a9d8993fc8ac67d6db9333","unresolved":false,"context_lines":[{"line_number":467,"context_line":"            network_info \u003d jsonutils.loads(network_info)"},{"line_number":468,"context_line":"        return cls([VIF.hydrate(vif) for vif in network_info])"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def wait(self, do_raise\u003dTrue):"},{"line_number":471,"context_line":"        \"\"\"A no-op method."},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        This is useful to avoid type-checking when NetworkInfo might be"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_da42f83d","line":470,"in_reply_to":"9a629dbe_be7e6bac","updated":"2016-11-09 07:35:45.000000000","message":"Yes that is the point, because that looks like a hack in few week somebody are going to push a new patch to remove that unnecessary method. Really it\u0027s not how we should fix it. What is the problem with isinstance ?","commit_id":"bf27b8d9984d9e3816c08d0443b5185e9960505d"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"0d5f0182904ac1a9324eba71662673851e0d7252","unresolved":false,"context_lines":[{"line_number":467,"context_line":"            network_info \u003d jsonutils.loads(network_info)"},{"line_number":468,"context_line":"        return cls([VIF.hydrate(vif) for vif in network_info])"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def wait(self, do_raise\u003dTrue):"},{"line_number":471,"context_line":"        \"\"\"A no-op method."},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        This is useful to avoid type-checking when NetworkInfo might be"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_99e16001","line":470,"in_reply_to":"9a629dbe_da42f83d","updated":"2016-11-09 13:26:18.000000000","message":"Performance is one, as I mentioned before. Also read http://canonical.org/~kragen/isinstance/ .\n\nThe PTL wanted it this way... see http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2016-11-03.log.html#t2016-11-03T14:01:46","commit_id":"bf27b8d9984d9e3816c08d0443b5185e9960505d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0745eee1ef85c3ba9005a8e2ec204d01337f3151","unresolved":false,"context_lines":[{"line_number":470,"context_line":"    def wait(self, do_raise\u003dTrue):"},{"line_number":471,"context_line":"        \"\"\"A no-op method."},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        This is useful to avoid type-checking when NetworkInfo might be"},{"line_number":474,"context_line":"        subclassed with NetworkInfoAsyncWrapper"},{"line_number":475,"context_line":"        \"\"\""},{"line_number":476,"context_line":"        pass"},{"line_number":477,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a629dbe_3bc11f55","line":474,"range":{"start_line":473,"start_character":8,"end_line":474,"end_character":47},"updated":"2016-11-09 16:47:06.000000000","message":"This could be more forceful or informative about why this is needed to guard against someone removing it accidentally.","commit_id":"85bf5cbf97bc2681cb7d115898dacbcab5d70801"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"045c02c5bb216ebe5c8008b603b2192c2ede4671","unresolved":false,"context_lines":[{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def wait(self, do_raise\u003dTrue):"},{"line_number":471,"context_line":"        \"\"\"Wait for asynchronous call to finish.\"\"\""},{"line_number":472,"context_line":"        # There is no asynchronous call for the base class, so this is a no-op"},{"line_number":473,"context_line":"        # here, but subclasses may override to provide asynchronous"},{"line_number":474,"context_line":"        # capabilities. Must be defined here in the parent class so that code"},{"line_number":475,"context_line":"        # which works with both parent and subclass types can reference this"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a629dbe_93f68ff7","line":472,"range":{"start_line":472,"start_character":10,"end_line":472,"end_character":58},"updated":"2016-11-09 18:33:24.000000000","message":"This is a tad bit confusing because I read this as the base class of this class, which would be list, which wouldn\u0027t be right.","commit_id":"706121156c699d2575f2345c2beba46c49e24a10"}]}