)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"9d00b7b7bcd284939c41f7fd5757821d796d49c9","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The method _process_port_bindings for the ml2 plugin should not"},{"line_number":10,"context_line":"return True when the host binding does not change, otherwise an"},{"line_number":11,"context_line":"unnecessary notification will be sent to the agent"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closed-Bug: #1255680"},{"line_number":14,"context_line":"Related-Bug: #1253896"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAATn%2F%2BBh4%3D","line":11,"updated":"2013-12-03 10:46:16.000000000","message":"I am not yet familiar with all the code, just a thought:\n\nright now _process_port_bindings is expected to signal \u0027completed processing successfully\u0027  - I guess - because it returns True at the end. \n\nThe suggestion here is to instead signal that \u0027port binding didn\u0027t change\u0027. \n\nI don\u0027t know if this causes issues - from a quick look at neutron/plugins/ml2/plugin.py seems to be invoked in create_port and update_port. In update_port the return value is used as need_port_update_notify. Is this ok?","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"a416c4759e42724f762f92bc2b308c74f1542378","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The method _process_port_bindings for the ml2 plugin should not"},{"line_number":10,"context_line":"return True when the host binding does not change, otherwise an"},{"line_number":11,"context_line":"unnecessary notification will be sent to the agent"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closed-Bug: #1255680"},{"line_number":14,"context_line":"Related-Bug: #1253896"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAATn%2F9cqc%3D","line":11,"in_reply_to":"AAAATn%2F%2BBh4%3D","updated":"2013-12-11 12:49:07.000000000","message":"The return value \"True\" has always been used to signify that the process completed successfully and something changed; False means success but nothing changed, and in case of error you\u0027ll have an exception.\n\nChanging the logic from positive to negative won\u0027t be a big deal.\n\nI see s downside to this proposal is consistency as most process_xxx methods adopt the same logic.","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"070076345df28dc0d6bb23deacda0daffef790d0","unresolved":false,"context_lines":[{"line_number":10,"context_line":"return True when the host binding does not change, otherwise an"},{"line_number":11,"context_line":"unnecessary notification will be sent to the agent"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closed-Bug: #1255680"},{"line_number":14,"context_line":"Related-Bug: #1253896"},{"line_number":15,"context_line":"Partially Implements: neutron-tempest-parallel"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAATn%2F%2BRKA%3D","line":13,"updated":"2013-11-28 11:46:07.000000000","message":"Closed -\u003e Closes","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"777c775fe247545b680a42416b405ca599fbe302","unresolved":false,"context_lines":[{"line_number":10,"context_line":"return True when the host binding does not change, otherwise an"},{"line_number":11,"context_line":"unnecessary notification will be sent to the agent"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closed-Bug: #1255680"},{"line_number":14,"context_line":"Related-Bug: #1253896"},{"line_number":15,"context_line":"Partially Implements: neutron-tempest-parallel"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAATn%2F%2BOk8%3D","line":13,"in_reply_to":"AAAATn%2F%2BRKA%3D","updated":"2013-11-28 16:24:17.000000000","message":"I think I\u0027ve made this mistake 200 times in the last 2 days","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"}],"neutron/plugins/ml2/plugin.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"070076345df28dc0d6bb23deacda0daffef790d0","unresolved":false,"context_lines":[{"line_number":217,"context_line":"        # Return True only if an agent notification is needed."},{"line_number":218,"context_line":"        # This will happen if a new host was specified and that host"},{"line_number":219,"context_line":"        # differs from the current one"},{"line_number":220,"context_line":"        ret_value \u003d host_set and binding.get(\u0027host\u0027) !\u003d host"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        if host_set:"},{"line_number":223,"context_line":"            binding.host \u003d host"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BPsA%3D","line":220,"updated":"2013-11-28 11:46:07.000000000","message":"Ah.. the logic is correct. I first thought if a previous value is an empty string \u0027\u0027 notification is not sent, but host_set is True even if \"host\" is empty. When _process_port_binding is called from update_port, host_id is always True. A bit confusing.","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"777c775fe247545b680a42416b405ca599fbe302","unresolved":false,"context_lines":[{"line_number":217,"context_line":"        # Return True only if an agent notification is needed."},{"line_number":218,"context_line":"        # This will happen if a new host was specified and that host"},{"line_number":219,"context_line":"        # differs from the current one"},{"line_number":220,"context_line":"        ret_value \u003d host_set and binding.get(\u0027host\u0027) !\u003d host"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        if host_set:"},{"line_number":223,"context_line":"            binding.host \u003d host"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BOko%3D","line":220,"in_reply_to":"AAAATn%2F%2BPsA%3D","updated":"2013-11-28 16:24:17.000000000","message":"Yes it is confusing and it took me a while to figure out how to proceed.","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"}],"neutron/tests/unit/ml2/test_port_binding.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"070076345df28dc0d6bb23deacda0daffef790d0","unresolved":false,"context_lines":[{"line_number":94,"context_line":"                                            {\u0027port\u0027: update_body},"},{"line_number":95,"context_line":"                                            neutron_context\u003dneutron_context)"},{"line_number":96,"context_line":"                port_data \u003d updated_port[\u0027port\u0027]"},{"line_number":97,"context_line":"                if new_host:"},{"line_number":98,"context_line":"                    self.assertEqual(port_data[\u0027binding:host_id\u0027], new_host)"},{"line_number":99,"context_line":"                else:"},{"line_number":100,"context_line":"                    self.assertEqual(port_data[\u0027binding:host_id\u0027], host)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BPqY%3D","line":97,"updated":"2013-11-28 11:46:07.000000000","message":"if tests commented below are added, this needs to be:\n\n  new_host is not None","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"},{"author":{"_account_id":333,"name":"Isaku Yamahata","email":"isaku.yamahata@gmail.com","username":"yamahata"},"change_message_id":"855d9942423bad30a8033e960f823beb997e0754","unresolved":false,"context_lines":[{"line_number":94,"context_line":"                                            {\u0027port\u0027: update_body},"},{"line_number":95,"context_line":"                                            neutron_context\u003dneutron_context)"},{"line_number":96,"context_line":"                port_data \u003d updated_port[\u0027port\u0027]"},{"line_number":97,"context_line":"                if new_host:"},{"line_number":98,"context_line":"                    self.assertEqual(port_data[\u0027binding:host_id\u0027], new_host)"},{"line_number":99,"context_line":"                else:"},{"line_number":100,"context_line":"                    self.assertEqual(port_data[\u0027binding:host_id\u0027], host)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BCHE%3D","line":97,"in_reply_to":"AAAATn%2F%2BPqY%3D","updated":"2013-12-03 08:30:12.000000000","message":"attributes.is_attr_set()?","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"070076345df28dc0d6bb23deacda0daffef790d0","unresolved":false,"context_lines":[{"line_number":98,"context_line":"                    self.assertEqual(port_data[\u0027binding:host_id\u0027], new_host)"},{"line_number":99,"context_line":"                else:"},{"line_number":100,"context_line":"                    self.assertEqual(port_data[\u0027binding:host_id\u0027], host)"},{"line_number":101,"context_line":"                if new_host and new_host !\u003d host:"},{"line_number":102,"context_line":"                    notify_mock.assert_called_once_with(mock.ANY)"},{"line_number":103,"context_line":"                else:"},{"line_number":104,"context_line":"                    self.assertFalse(notify_mock.called)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BPqU%3D","line":101,"updated":"2013-11-28 11:46:07.000000000","message":"Same as the above.","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"070076345df28dc0d6bb23deacda0daffef790d0","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                                       \u0027host-ovs-no-filter\u0027)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_update_without_binding_does_not_notify(self):"},{"line_number":115,"context_line":"        self._test_update_port_binding(\u0027host-ovs-no-filter\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BPq0%3D","line":115,"updated":"2013-11-28 11:46:07.000000000","message":"Could you add tests to check notification is sent when host_id changes from empty to non-empty and vice versa.\nAs I commented in plugin.py, it is a bit tricky and worth covered by UT.\n\n  def test_update_from_empty_to_host_binding_notifies_agent(self):\n    self._test_update_port_binding(\u0027\u0027, \u0027host-ovs-no-filter\u0027)\n\n  def test_update_from_host_to_empty_binding_notifies_agent(self):\n    self._test_update_port_binding(\u0027host-ovs-no-filter\u0027, \u0027\u0027)","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"},{"author":{"_account_id":333,"name":"Isaku Yamahata","email":"isaku.yamahata@gmail.com","username":"yamahata"},"change_message_id":"855d9942423bad30a8033e960f823beb997e0754","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                                       \u0027host-ovs-no-filter\u0027)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_update_without_binding_does_not_notify(self):"},{"line_number":115,"context_line":"        self._test_update_port_binding(\u0027host-ovs-no-filter\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BCHI%3D","line":115,"in_reply_to":"AAAATn%2F%2BOkg%3D","updated":"2013-12-03 08:30:12.000000000","message":"+ ATTR_NOT_SPECIFIED?","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"777c775fe247545b680a42416b405ca599fbe302","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                                       \u0027host-ovs-no-filter\u0027)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_update_without_binding_does_not_notify(self):"},{"line_number":115,"context_line":"        self._test_update_port_binding(\u0027host-ovs-no-filter\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BOkg%3D","line":115,"in_reply_to":"AAAATn%2F%2BPq0%3D","updated":"2013-11-28 16:24:17.000000000","message":"will do","commit_id":"8554744f081a31bd6ae6af9e1bb2883235c49465"}]}
