)]}'
{".gitignore":[{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"721d802ff5d4abf4924c1747d1675e0d2907da54","unresolved":true,"context_lines":[{"line_number":57,"context_line":"ChangeLog"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"# Files created by releasenotes build"},{"line_number":60,"context_line":"releasenotes/build"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"175c57f8_8c8d4a37","line":60,"updated":"2024-08-02 14:57:40.000000000","message":"this file should not be here","commit_id":"01e7b1be00a680ede6bd4fe3e7f17e8b1cded32d"}],"/COMMIT_MSG":[{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"bf626406a654141789a05d9d5faf515d08c8b193","unresolved":true,"context_lines":[{"line_number":1,"context_line":"Parent:     b5d86937 ([OVN][CI] Update OVS_BRANCH to be compatible with ovn main)"},{"line_number":2,"context_line":"Author:     Pierre Riteau \u003cpierre@stackhpc.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2024-07-29 15:16:30 +0200"},{"line_number":4,"context_line":"Commit:     Chris Buggy \u003ccbuggy@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"cb0bdce3_c4f52eb2","line":1,"updated":"2024-08-21 10:23:36.000000000","message":"How did this commit ended up authored by me?","commit_id":"55f84d4f74308a139b0617d0fd9c4c206d7fbfc9"},{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"8ca1f407d4bc4fe89fcd4c943c906bf7d0dc0d51","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     b5d86937 ([OVN][CI] Update OVS_BRANCH to be compatible with ovn main)"},{"line_number":2,"context_line":"Author:     Pierre Riteau \u003cpierre@stackhpc.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2024-07-29 15:16:30 +0200"},{"line_number":4,"context_line":"Commit:     Chris Buggy \u003ccbuggy@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"22c9d796_b5c5641a","line":1,"in_reply_to":"cb0bdce3_c4f52eb2","updated":"2024-08-21 10:29:07.000000000","message":"Done","commit_id":"55f84d4f74308a139b0617d0fd9c4c206d7fbfc9"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"0f2edc44d73b398bf96dbc536001ee8c038ef5f8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"de08ceb5_3b840f2f","updated":"2024-07-23 14:02:40.000000000","message":"Code looks ok, I don\u0027t know if repo mantainers would like some testing","commit_id":"a5a6aa0cb8c36a31cd778c068f00041a4244fedb"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"2bff9833c85829c39d47bb210325b6fcc1470c1e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"55b41d54_77953b77","updated":"2024-07-23 15:05:21.000000000","message":"Yeah, code looks ok but would need some unit tests to come back coverage job over the threshold","commit_id":"a5a6aa0cb8c36a31cd778c068f00041a4244fedb"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"721d802ff5d4abf4924c1747d1675e0d2907da54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"3b7bdd5e_4dcf4dcf","updated":"2024-08-02 14:57:40.000000000","message":"Some minor comments","commit_id":"01e7b1be00a680ede6bd4fe3e7f17e8b1cded32d"},{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"8ca1f407d4bc4fe89fcd4c943c906bf7d0dc0d51","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"288e5733_597c8408","updated":"2024-08-21 10:29:07.000000000","message":"Sorry for interrupting the merged, but I fixed the Git author for this commit.","commit_id":"efd63d1721742400e7ba2c0bfc55249ef15fc549"}],"ovn_octavia_provider/helper.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"fd6299f5bed296f9038add70a53fc58a68e90073","unresolved":true,"context_lines":[{"line_number":107,"context_line":"            for k, v in status.items()}"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def _check_and_set_ssl_files(self):"},{"line_number":110,"context_line":"        # TODO(reedip): Make ovsdb_monitor\u0027s _check_and_set_ssl_files() public"},{"line_number":111,"context_line":"        # This is a copy of ovsdb_monitor._check_and_set_ssl_files"},{"line_number":112,"context_line":"        priv_key_file \u003d ovn_conf.get_ovn_nb_private_key()"},{"line_number":113,"context_line":"        cert_file \u003d ovn_conf.get_ovn_nb_certificate()"},{"line_number":114,"context_line":"        ca_cert_file \u003d ovn_conf.get_ovn_nb_ca_cert()"}],"source_content_type":"text/x-python","patch_set":2,"id":"64985d21_61c6b35f","line":111,"range":{"start_line":110,"start_character":8,"end_line":111,"end_character":66},"updated":"2024-07-15 15:08:44.000000000","message":"Do it once. Refactor the ovsdb_monitor method (make it public) and use it.","commit_id":"9f0b5a2ddc62d41b6a5eb0a4e2eb6d105962057c"}],"ovn_octavia_provider/ovsdb/ovsdb_monitor.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"fd6299f5bed296f9038add70a53fc58a68e90073","unresolved":true,"context_lines":[{"line_number":85,"context_line":"        self.driver \u003d driver"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"def _check_and_set_ssl_files(schema_name):"},{"line_number":89,"context_line":"    if schema_name \u003d\u003d \u0027OVN_Northbound\u0027:"},{"line_number":90,"context_line":"        priv_key_file \u003d ovn_config.get_ovn_nb_private_key()"},{"line_number":91,"context_line":"        cert_file \u003d ovn_config.get_ovn_nb_certificate()"}],"source_content_type":"text/x-python","patch_set":2,"id":"5709a251_a04caeef","line":88,"range":{"start_line":88,"start_character":4,"end_line":88,"end_character":28},"updated":"2024-07-15 15:08:44.000000000","message":"Too much code duplicated, please refactor this method as is implemented in Neutron: https://github.com/openstack/neutron/blob/6450d3066727ad25be1380b31ff335cbafdf7131/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py#L941","commit_id":"9f0b5a2ddc62d41b6a5eb0a4e2eb6d105962057c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"fd6299f5bed296f9038add70a53fc58a68e90073","unresolved":true,"context_lines":[{"line_number":90,"context_line":"        priv_key_file \u003d ovn_config.get_ovn_nb_private_key()"},{"line_number":91,"context_line":"        cert_file \u003d ovn_config.get_ovn_nb_certificate()"},{"line_number":92,"context_line":"        ca_cert_file \u003d ovn_config.get_ovn_nb_ca_cert()"},{"line_number":93,"context_line":"        if priv_key_file:"},{"line_number":94,"context_line":"            Stream.ssl_set_private_key_file(priv_key_file)"},{"line_number":95,"context_line":"        else:"},{"line_number":96,"context_line":"            LOG.error(\"Cannot find private key file for northbound db\")"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        if cert_file:"},{"line_number":99,"context_line":"            Stream.ssl_set_certificate_file(cert_file)"},{"line_number":100,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"33bf9aff_3cce8e5c","line":97,"range":{"start_line":93,"start_character":8,"end_line":97,"end_character":1},"updated":"2024-07-15 15:08:44.000000000","message":"This is incorrect (same comment applies to any other config option): the user can define any of these config options as empty (the default value is \u0027\u0027).\n\nWhat you need to check is that if any of these values is something different to an empty string, check if the file actually exists:\n```\nif priv_key_file:\n    if not os.path.exists(priv_key_file)\n        LOG.error(\"Cannot find private key file for northbound db\")\n    else:\n        Stream.ssl_set_private_key_file(priv_key_file)\n\n```","commit_id":"9f0b5a2ddc62d41b6a5eb0a4e2eb6d105962057c"}],"ovn_octavia_provider/tests/unit/ovsdb/test_ovsdb_monitor.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ec381804069496e55cb7d8c443dc9d854c1360ed","unresolved":true,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class TestOvsdbMonitor(base.BaseTestCase):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    @mock.patch(\u0027ovn_octavia_provider.ovsdb.ovsdb_monitor.os.path\u0027)"},{"line_number":24,"context_line":"    @mock.patch(\u0027ovn_octavia_provider.ovsdb.ovsdb_monitor.ovn_config\u0027)"},{"line_number":25,"context_line":"    @mock.patch(\u0027ovn_octavia_provider.ovsdb.ovsdb_monitor.Stream\u0027)"},{"line_number":26,"context_line":"    def test_set_nb_ssl(self, mock_stream, mock_conf, mock_os_path):"}],"source_content_type":"text/x-python","patch_set":4,"id":"d2002bba_9ee862c9","line":23,"updated":"2024-07-31 11:25:39.000000000","message":"Some comments here to improve the code:\n* SB and NB can implement same testing methods just changing the SCHEMA_NAME and the method names.\n* There is no need to check, when using for example NB, that the SB methods have not been called.\n* It is better to use @mock.patch.object(module, \u0027object\u0027), instead of using the path. That will load the module that implements the object to mock.\n* You should mock the libraries, not the imported references. For example: @mock.patch.object(os, \u0027path\u0027)","commit_id":"f7c568313f956aa2307b1854c573be8769f914b2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2f45e3c582ebd68c5972869ff5e9a9cd2a02a0b5","unresolved":true,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def setUp(self):"},{"line_number":48,"context_line":"        super().setUp()"},{"line_number":49,"context_line":"        ovn_conf.register_opts()"},{"line_number":50,"context_line":"        self.mock_os_path \u003d mock.patch(\u0027os.path.exists\u0027).start()"},{"line_number":51,"context_line":"        self.mock_stream \u003d mock.patch.object(ovsdb_monitor, \u0027Stream\u0027).start()"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"eb739231_d581e4aa","line":49,"range":{"start_line":49,"start_character":8,"end_line":49,"end_character":32},"updated":"2024-08-02 10:23:34.000000000","message":"This is failing because of this:\n  oslo_config.cfg.DuplicateOptError: duplicate option: ovn_nb_connection\n  \nI don\u0027t know if the parent base class is loading any of the config options needed. In that case, create a method to import the config options one by one:\n```\nOPTS \u003d (\u0027ovn_nb_private_key\u0027, \u0027ovn_nb_certificate\u0027, \u0027ovn_nb_ca_cert\u0027,\n        \u0027ovn_sb_private_key\u0027, \u0027ovn_sb_certificate\u0027, \u0027ovn_sb_ca_cert\u0027)\ndef _register_opts()\n    for opt in OPTS:\n        try:\n            cfg.CONF.register_opt(cfg.StrOpt(opt), group\u003d\u0027ovn\u0027)\n        except oslo_config.cfg.DuplicateOptError:\n            pass\n```","commit_id":"34dd78111ecdf8ecd0fe12850e6976efc8ef4fc8"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"721d802ff5d4abf4924c1747d1675e0d2907da54","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        cfg.CONF.set_override(self.ca_cert, \u0027ca-cert\u0027, group\u003d\u0027ovn\u0027)"},{"line_number":61,"context_line":"        self.mock_os_path.return_value \u003d True"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        with self.assertNoLogs():"},{"line_number":64,"context_line":"            ovsdb_monitor.check_and_set_ssl_files(self.schema)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        self.mock_stream.ssl_set_private_key_file.assert_called_with(\u0027key\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"78bfc28f_90b97621","line":63,"updated":"2024-08-02 14:57:40.000000000","message":"looks like assertNoLogs is added in python 3.10 [1]\n\n[1] https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertNoLogs","commit_id":"01e7b1be00a680ede6bd4fe3e7f17e8b1cded32d"}]}