)]}'
{"networking_ovn/db/hash_ring.py":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"3e83deb63571cb5b894118d93f16d53375f52e27","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def _touch(hostname\u003dNone, node_uuid\u003dNone):"},{"line_number":47,"context_line":"    filter_args \u003d {}"},{"line_number":48,"context_line":"    if hostname is not None:"},{"line_number":49,"context_line":"        filter_args[\u0027hostname\u0027] \u003d hostname"},{"line_number":50,"context_line":"    if node_uuid is not None:"},{"line_number":51,"context_line":"        filter_args[\u0027node_uuid\u0027] \u003d node_uuid"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_dfe7a3ab","line":48,"range":{"start_line":48,"start_character":4,"end_line":48,"end_character":28},"updated":"2019-06-27 15:03:36.000000000","message":"nit: I like \"if not hostname\" :)","commit_id":"c3f93c909f7f6eb9370e6eafe58e5439c80d61c1"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"7fbea21ec572cc88d4005c96e0bf346fb120e575","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def _touch(hostname\u003dNone, node_uuid\u003dNone):"},{"line_number":47,"context_line":"    filter_args \u003d {}"},{"line_number":48,"context_line":"    if hostname is not None:"},{"line_number":49,"context_line":"        filter_args[\u0027hostname\u0027] \u003d hostname"},{"line_number":50,"context_line":"    if node_uuid is not None:"},{"line_number":51,"context_line":"        filter_args[\u0027node_uuid\u0027] \u003d node_uuid"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_a2141845","line":48,"range":{"start_line":48,"start_character":4,"end_line":48,"end_character":28},"in_reply_to":"9fb8cfa7_3f683ff3","updated":"2019-06-27 15:13:38.000000000","message":"Got it! Leave it like this then, makes sense","commit_id":"c3f93c909f7f6eb9370e6eafe58e5439c80d61c1"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f58a41d53798c8861e172fcd04591e6143df4660","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def _touch(hostname\u003dNone, node_uuid\u003dNone):"},{"line_number":47,"context_line":"    filter_args \u003d {}"},{"line_number":48,"context_line":"    if hostname is not None:"},{"line_number":49,"context_line":"        filter_args[\u0027hostname\u0027] \u003d hostname"},{"line_number":50,"context_line":"    if node_uuid is not None:"},{"line_number":51,"context_line":"        filter_args[\u0027node_uuid\u0027] \u003d node_uuid"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_3f683ff3","line":48,"range":{"start_line":48,"start_character":4,"end_line":48,"end_character":28},"in_reply_to":"9fb8cfa7_dfe7a3ab","updated":"2019-06-27 15:09:40.000000000","message":"It\u0027s just good practice to test the exact value of the placeholder (None in this case).\n\nI don\u0027t think it has any benefits in this method tho so I will remove it as u suggested. But for other cases, say someone invokes this with hostname\u003d\u0027\u0027 as a parameter, the \"if not hostname\" would return False since it\u0027s an empty string but in some cases empty strings are valid values.","commit_id":"c3f93c909f7f6eb9370e6eafe58e5439c80d61c1"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"3e83deb63571cb5b894118d93f16d53375f52e27","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    filter_args \u003d {}"},{"line_number":48,"context_line":"    if hostname is not None:"},{"line_number":49,"context_line":"        filter_args[\u0027hostname\u0027] \u003d hostname"},{"line_number":50,"context_line":"    if node_uuid is not None:"},{"line_number":51,"context_line":"        filter_args[\u0027node_uuid\u0027] \u003d node_uuid"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    session \u003d db_api.get_writer_session()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_ffeaa771","line":50,"range":{"start_line":50,"start_character":7,"end_line":50,"end_character":28},"updated":"2019-06-27 15:03:36.000000000","message":"same","commit_id":"c3f93c909f7f6eb9370e6eafe58e5439c80d61c1"}],"networking_ovn/ovsdb/ovsdb_monitor.py":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"3e83deb63571cb5b894118d93f16d53375f52e27","unresolved":false,"context_lines":[{"line_number":371,"context_line":"            seconds\u003dconstants.HASH_RING_TOUCH_INTERVAL)"},{"line_number":372,"context_line":"        if not self._last_touch or touch_timeout \u003e\u003d self._last_touch:"},{"line_number":373,"context_line":"            db_hash_ring.touch_node(self._node_uuid)"},{"line_number":374,"context_line":"            self._last_touch \u003d timeutils.utcnow()"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        LOG.debug(\u0027Hash Ring: Node %(node)s (host: %(hostname)s) \u0027"},{"line_number":377,"context_line":"                  \u0027handling event \"%(event)s\" for row %(row)s \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_9f8a8bbc","line":374,"range":{"start_line":374,"start_character":31,"end_line":374,"end_character":49},"updated":"2019-06-27 15:03:36.000000000","message":"Maybe we can get timeutils.utcnow() before L370 and use it in both places? IT won\u0027t make a big difference though","commit_id":"c3f93c909f7f6eb9370e6eafe58e5439c80d61c1"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f58a41d53798c8861e172fcd04591e6143df4660","unresolved":false,"context_lines":[{"line_number":371,"context_line":"            seconds\u003dconstants.HASH_RING_TOUCH_INTERVAL)"},{"line_number":372,"context_line":"        if not self._last_touch or touch_timeout \u003e\u003d self._last_touch:"},{"line_number":373,"context_line":"            db_hash_ring.touch_node(self._node_uuid)"},{"line_number":374,"context_line":"            self._last_touch \u003d timeutils.utcnow()"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        LOG.debug(\u0027Hash Ring: Node %(node)s (host: %(hostname)s) \u0027"},{"line_number":377,"context_line":"                  \u0027handling event \"%(event)s\" for row %(row)s \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_5f7e53af","line":374,"range":{"start_line":374,"start_character":31,"end_line":374,"end_character":49},"in_reply_to":"9fb8cfa7_9f8a8bbc","updated":"2019-06-27 15:09:40.000000000","message":"++ good point!","commit_id":"c3f93c909f7f6eb9370e6eafe58e5439c80d61c1"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"08454e313b91cabc315d79939c73273e67595f18","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                db_hash_ring.touch_node(self._node_uuid)"},{"line_number":379,"context_line":"                self._last_touch \u003d time_now"},{"line_number":380,"context_line":"            except Exception as e:"},{"line_number":381,"context_line":"                LOG.exception(\u0027Hash Ring node %s failed to heartbeat\u0027)"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"        LOG.debug(\u0027Hash Ring: Node %(node)s (host: %(hostname)s) \u0027"},{"line_number":384,"context_line":"                  \u0027handling event \"%(event)s\" for row %(row)s \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_b4dc4ee4","line":381,"range":{"start_line":381,"start_character":46,"end_line":381,"end_character":48},"updated":"2019-06-27 20:30:29.000000000","message":"forgot the node_uuid here T.T","commit_id":"3d40b691bef5d86f0fad7850f4f45258af251590"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"52ee18959b3b6129827efe9dbec81068a1fdb11b","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        # If the worker hasn\u0027t been health checked by the maintenance"},{"line_number":369,"context_line":"        # thread (see bug #1834498), indicate that it\u0027s alive here"},{"line_number":370,"context_line":"        time_now \u003d timeutils.utcnow()"},{"line_number":371,"context_line":"        touch_timeout \u003d time_now - datetime.timedelta("},{"line_number":372,"context_line":"            seconds\u003dovn_const.HASH_RING_TOUCH_INTERVAL)"},{"line_number":373,"context_line":"        if not self._last_touch or touch_timeout \u003e\u003d self._last_touch:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_c0281234","line":370,"updated":"2019-06-28 15:25:15.000000000","message":"Do you want to test this code with unittests perhaps? I spent some time thinking about L373 and figured out comparisons with None behave differently on Python 2 and Python 3. So perhaps it would be great to cover it with unittests.","commit_id":"9b383ebeb77e911a0f5834f988dbdefd54a50e7d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"db8e17e56388ebbd125e473e03602ae9c13725bc","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        # If the worker hasn\u0027t been health checked by the maintenance"},{"line_number":369,"context_line":"        # thread (see bug #1834498), indicate that it\u0027s alive here"},{"line_number":370,"context_line":"        time_now \u003d timeutils.utcnow()"},{"line_number":371,"context_line":"        touch_timeout \u003d time_now - datetime.timedelta("},{"line_number":372,"context_line":"            seconds\u003dovn_const.HASH_RING_TOUCH_INTERVAL)"},{"line_number":373,"context_line":"        if not self._last_touch or touch_timeout \u003e\u003d self._last_touch:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_809f7a4e","line":370,"in_reply_to":"9fb8cfa7_c0281234","updated":"2019-06-28 15:30:56.000000000","message":"Sounds good! I can do some work on that","commit_id":"9b383ebeb77e911a0f5834f988dbdefd54a50e7d"}],"networking_ovn/tests/unit/ovsdb/test_ovsdb_monitor.py":[{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"b016df297cf2688c9b20377b264d69b7e9807650","unresolved":false,"context_lines":[{"line_number":452,"context_line":"            attrs\u003d{\u0027_table\u0027: mock.Mock(name\u003d\u0027FakeTable\u0027)})"},{"line_number":453,"context_line":""},{"line_number":454,"context_line":"    @mock.patch.object(db_hash_ring, \u0027touch_node\u0027)"},{"line_number":455,"context_line":"    @mock.patch.object(hash_ring_manager.HashRingManager, \u0027get_node\u0027)"},{"line_number":456,"context_line":"    def test_notify(self, mock_get_node, mock_touch_node):"},{"line_number":457,"context_line":"        mock_get_node.return_value \u003d self.node_uuid"},{"line_number":458,"context_line":"        self.idl.notify(self.fake_event, self.fake_row)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fb8cfa7_c40a5388","line":455,"range":{"start_line":455,"start_character":0,"end_line":455,"end_character":69},"updated":"2019-07-01 10:09:45.000000000","message":"can we move this patch to setUp?","commit_id":"c5936dc1c0907b7bc485eaa974a0babf6aaf724b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8c295127225ca7a554fb1b9b62889c397d89ec20","unresolved":false,"context_lines":[{"line_number":452,"context_line":"            attrs\u003d{\u0027_table\u0027: mock.Mock(name\u003d\u0027FakeTable\u0027)})"},{"line_number":453,"context_line":""},{"line_number":454,"context_line":"    @mock.patch.object(db_hash_ring, \u0027touch_node\u0027)"},{"line_number":455,"context_line":"    @mock.patch.object(hash_ring_manager.HashRingManager, \u0027get_node\u0027)"},{"line_number":456,"context_line":"    def test_notify(self, mock_get_node, mock_touch_node):"},{"line_number":457,"context_line":"        mock_get_node.return_value \u003d self.node_uuid"},{"line_number":458,"context_line":"        self.idl.notify(self.fake_event, self.fake_row)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fb8cfa7_c47d73e7","line":455,"range":{"start_line":455,"start_character":0,"end_line":455,"end_character":69},"in_reply_to":"9fb8cfa7_c40a5388","updated":"2019-07-01 10:15:05.000000000","message":"Good point, we can indeed :D","commit_id":"c5936dc1c0907b7bc485eaa974a0babf6aaf724b"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"d0dd70046e3103788c0e9346bfd158cc59ce35cb","unresolved":false,"context_lines":[{"line_number":452,"context_line":"            attrs\u003d{\u0027_table\u0027: mock.Mock(name\u003d\u0027FakeTable\u0027)})"},{"line_number":453,"context_line":""},{"line_number":454,"context_line":"    @mock.patch.object(db_hash_ring, \u0027touch_node\u0027)"},{"line_number":455,"context_line":"    @mock.patch.object(hash_ring_manager.HashRingManager, \u0027get_node\u0027)"},{"line_number":456,"context_line":"    def test_notify(self, mock_get_node, mock_touch_node):"},{"line_number":457,"context_line":"        mock_get_node.return_value \u003d self.node_uuid"},{"line_number":458,"context_line":"        self.idl.notify(self.fake_event, self.fake_row)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fb8cfa7_a4bf1f5f","line":455,"range":{"start_line":455,"start_character":0,"end_line":455,"end_character":69},"in_reply_to":"9fb8cfa7_c47d73e7","updated":"2019-07-01 10:28:31.000000000","message":"thx :)","commit_id":"c5936dc1c0907b7bc485eaa974a0babf6aaf724b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5ad93854b34c7b07e00eb40f5be33f92202c76ae","unresolved":false,"context_lines":[{"line_number":430,"context_line":"        self.assertTrue(self.event.match_fn(ROW_UPDATE, row, old\u003dold))"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"@mock.patch.object(hash_ring_manager.HashRingManager, \u0027get_node\u0027)"},{"line_number":434,"context_line":"class TestOvnIdlDistributedLock(base.TestCase):"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fb8cfa7_7aa53c02","line":433,"range":{"start_line":433,"start_character":0,"end_line":433,"end_character":65},"updated":"2019-07-01 11:50:06.000000000","message":"No action required, just a recommendation:\nif you move this into setUp() then you can avoid setting the return_value in other test methods:\n\n self.mock_get_node \u003d mock.patch.object(\n     hash_ring_manager.HashRingManager, \u0027get_node\u0027, \n     return_value\u003dself.node_uuid).start()\n\nthen you can remove L455, L466, L478 and L497.\n\nBut it\u0027s really just another way how to write it :) I don\u0027t insist if you prefer the way it is right now.","commit_id":"db5608ce7064460a54b6a36022728f5c3c4a9313"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5ad93854b34c7b07e00eb40f5be33f92202c76ae","unresolved":false,"context_lines":[{"line_number":500,"context_line":"        # Assert that in an eventual failure on touch_node() the event"},{"line_number":501,"context_line":"        # will continue to be processed by notify_handler.notify()"},{"line_number":502,"context_line":"        mock_touch_node.assert_called_once_with(self.node_uuid)"},{"line_number":503,"context_line":"        mock_log.assert_called_once_with("},{"line_number":504,"context_line":"            \u0027Hash Ring node %s failed to heartbeat\u0027, self.node_uuid)"},{"line_number":505,"context_line":"        self.idl.notify_handler.notify.assert_called_once_with("},{"line_number":506,"context_line":"            self.fake_event, self.fake_row, None)"},{"line_number":507,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9fb8cfa7_1ade806c","line":504,"range":{"start_line":503,"start_character":0,"end_line":504,"end_character":68},"updated":"2019-07-01 11:50:06.000000000","message":"Nit: I\u0027d avoid asserting on particular string just for cases when we want to update the error message we don\u0027t need to update this unittest. :) From my past experience people get triggered when they need to do it.","commit_id":"db5608ce7064460a54b6a36022728f5c3c4a9313"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"6f9bcc8edfeff79b25e976e864f27ea042f782da","unresolved":false,"context_lines":[{"line_number":500,"context_line":"        # Assert that in an eventual failure on touch_node() the event"},{"line_number":501,"context_line":"        # will continue to be processed by notify_handler.notify()"},{"line_number":502,"context_line":"        mock_touch_node.assert_called_once_with(self.node_uuid)"},{"line_number":503,"context_line":"        mock_log.assert_called_once_with("},{"line_number":504,"context_line":"            \u0027Hash Ring node %s failed to heartbeat\u0027, self.node_uuid)"},{"line_number":505,"context_line":"        self.idl.notify_handler.notify.assert_called_once_with("},{"line_number":506,"context_line":"            self.fake_event, self.fake_row, None)"},{"line_number":507,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9fb8cfa7_3a690424","line":504,"range":{"start_line":503,"start_character":0,"end_line":504,"end_character":68},"in_reply_to":"9fb8cfa7_1ade806c","updated":"2019-07-01 12:23:21.000000000","message":"It\u0027s a good point, perhaps just checking if the LOG.exception was called would be enough or just ignore it ?","commit_id":"db5608ce7064460a54b6a36022728f5c3c4a9313"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5dcef351bf8800f084811ba4f80f8be8849b145d","unresolved":false,"context_lines":[{"line_number":500,"context_line":"        # Assert that in an eventual failure on touch_node() the event"},{"line_number":501,"context_line":"        # will continue to be processed by notify_handler.notify()"},{"line_number":502,"context_line":"        mock_touch_node.assert_called_once_with(self.node_uuid)"},{"line_number":503,"context_line":"        mock_log.assert_called_once_with("},{"line_number":504,"context_line":"            \u0027Hash Ring node %s failed to heartbeat\u0027, self.node_uuid)"},{"line_number":505,"context_line":"        self.idl.notify_handler.notify.assert_called_once_with("},{"line_number":506,"context_line":"            self.fake_event, self.fake_row, None)"},{"line_number":507,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9fb8cfa7_bacbb41b","line":504,"range":{"start_line":503,"start_character":0,"end_line":504,"end_character":68},"in_reply_to":"9fb8cfa7_3a690424","updated":"2019-07-01 12:25:14.000000000","message":"if it\u0027s called sounds good","commit_id":"db5608ce7064460a54b6a36022728f5c3c4a9313"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"a5dba8aaa69185c26f7893a6fac4f587f3741f27","unresolved":false,"context_lines":[{"line_number":450,"context_line":"        patcher \u003d mock.patch.object(hash_ring_manager.HashRingManager,"},{"line_number":451,"context_line":"                                    \u0027get_node\u0027, return_value\u003dself.node_uuid)"},{"line_number":452,"context_line":"        self.mock_get_node \u003d patcher.start()"},{"line_number":453,"context_line":"        self.addCleanup(patcher.stop)"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"    @mock.patch.object(db_hash_ring, \u0027touch_node\u0027)"},{"line_number":456,"context_line":"    def test_notify(self, mock_touch_node):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fb8cfa7_faffecfe","line":453,"range":{"start_line":453,"start_character":0,"end_line":453,"end_character":37},"updated":"2019-07-01 12:37:15.000000000","message":"You don\u0027t need this anymore :) https://github.com/openstack/networking-ovn/blob/e0314b737c3b2c0287ed7cd441902bbe89d9def8/networking_ovn/tests/base.py#L48","commit_id":"48259a55a2b06ffa5191dc9b9851a74a4c709b28"}]}