)]}'
{"watcher/conf/_opts.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"705ccfd19c492722da8532d8ed4d1b1c4094f98e","unresolved":false,"context_lines":[{"line_number":56,"context_line":"         conf_ceilometer_client.CEILOMETER_CLIENT_OPTS),"},{"line_number":57,"context_line":"        (conf_neutron_client.neutron_client,"},{"line_number":58,"context_line":"         conf_neutron_client.NEUTRON_CLIENT_OPTS),"},{"line_number":59,"context_line":"        (conf_auth.WATCHER_CLIENTS_AUTH,"},{"line_number":60,"context_line":"         (ka_loading.get_auth_common_conf_options() +"},{"line_number":61,"context_line":"          ka_loading.get_auth_plugin_conf_options(\u0027password\u0027) +"},{"line_number":62,"context_line":"          ka_loading.get_session_conf_options()))"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_02dd006f","line":59,"range":{"start_line":59,"start_character":19,"end_line":59,"end_character":39},"updated":"2019-06-26 12:36:06.000000000","message":"I think this and DEFAULT are the only groups in list_opts that are strings rather than an OptGroup object.\n\nIf the long-term goal is to make sure that we use the OptGroup in list_opts rather than the just the name string of the group, should we have a test which asserts that everything in list_opts is an OptGroup except for the two known cases noted above? For example, I had to change a couple of tests in https://review.opendev.org/#/c/666897/2/watcher/tests/conf/test_list_opts.py to handle name (string) or OptGroup types - should we just assert that if it\u0027s a string it has to be either DEFAULT or conf_auth.WATCHER_CLIENTS_AUTH and fail if it\u0027s not to avoid new group registrations that aren\u0027t using the OptGroup (if you want to enforce consistency)?","commit_id":"47c8dbff2af371bcefa6f92bd8d9a458e90da2d2"},{"author":{"_account_id":29911,"name":"Dantali0n","email":"info@dantalion.nl","username":"Dantali0n"},"change_message_id":"2fb050770af31d5c7f926875c1d9ee37084011e9","unresolved":false,"context_lines":[{"line_number":56,"context_line":"         conf_ceilometer_client.CEILOMETER_CLIENT_OPTS),"},{"line_number":57,"context_line":"        (conf_neutron_client.neutron_client,"},{"line_number":58,"context_line":"         conf_neutron_client.NEUTRON_CLIENT_OPTS),"},{"line_number":59,"context_line":"        (conf_auth.WATCHER_CLIENTS_AUTH,"},{"line_number":60,"context_line":"         (ka_loading.get_auth_common_conf_options() +"},{"line_number":61,"context_line":"          ka_loading.get_auth_plugin_conf_options(\u0027password\u0027) +"},{"line_number":62,"context_line":"          ka_loading.get_session_conf_options()))"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_62295c35","line":59,"range":{"start_line":59,"start_character":19,"end_line":59,"end_character":39},"in_reply_to":"9fb8cfa7_02dd006f","updated":"2019-06-26 14:13:25.000000000","message":"Done","commit_id":"47c8dbff2af371bcefa6f92bd8d9a458e90da2d2"}],"watcher/conf/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"705ccfd19c492722da8532d8ed4d1b1c4094f98e","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from oslo_config import cfg"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"api \u003d cfg.OptGroup(name\u003d\u0027api\u0027,"},{"line_number":22,"context_line":"                   title\u003d\u0027Options for the Watcher API service\u0027)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"AUTH_OPTS \u003d ["}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_c25fe80c","side":"PARENT","line":21,"range":{"start_line":21,"start_character":19,"end_line":21,"end_character":24},"updated":"2019-06-26 12:36:06.000000000","message":"Why drop this? You didn\u0027t do it here for example:\n\nhttps://review.opendev.org/#/c/667522/2/watcher/conf/applier.py@21","commit_id":"5f521471e1e886a27fc8e722729812928f3dd8b2"},{"author":{"_account_id":29911,"name":"Dantali0n","email":"info@dantalion.nl","username":"Dantali0n"},"change_message_id":"2fb050770af31d5c7f926875c1d9ee37084011e9","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from oslo_config import cfg"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"api \u003d cfg.OptGroup(name\u003d\u0027api\u0027,"},{"line_number":22,"context_line":"                   title\u003d\u0027Options for the Watcher API service\u0027)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"AUTH_OPTS \u003d ["}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_a233d4aa","side":"PARENT","line":21,"range":{"start_line":21,"start_character":19,"end_line":21,"end_character":24},"in_reply_to":"9fb8cfa7_c25fe80c","updated":"2019-06-26 14:13:25.000000000","message":"Yes, no reason to change this. It is an artifact of me resolving all options being added twice to each group but that turned out to be due to _opts.py","commit_id":"5f521471e1e886a27fc8e722729812928f3dd8b2"}],"watcher/conf/decision_engine.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"705ccfd19c492722da8532d8ed4d1b1c4094f98e","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"def list_opts():"},{"line_number":90,"context_line":"    return [(watcher_decision_engine, WATCHER_DECISION_ENGINE_OPTS),"},{"line_number":91,"context_line":"            (watcher_decision_engine, WATCHER_CONTINUOUS_OPTS)]"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_e2b1cc45","line":90,"updated":"2019-06-26 12:36:06.000000000","message":"Seems kind of weird that we have a separate WATCHER_CONTINUOUS_OPTS list but it\u0027s in the same watcher_decision_engine group, but it\u0027s not a problem.","commit_id":"47c8dbff2af371bcefa6f92bd8d9a458e90da2d2"},{"author":{"_account_id":29911,"name":"Dantali0n","email":"info@dantalion.nl","username":"Dantali0n"},"change_message_id":"2fb050770af31d5c7f926875c1d9ee37084011e9","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"def list_opts():"},{"line_number":90,"context_line":"    return [(watcher_decision_engine, WATCHER_DECISION_ENGINE_OPTS),"},{"line_number":91,"context_line":"            (watcher_decision_engine, WATCHER_CONTINUOUS_OPTS)]"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_48955731","line":90,"in_reply_to":"9fb8cfa7_e2b1cc45","updated":"2019-06-26 14:13:25.000000000","message":"I think it is indeed best to clean this up as well, done","commit_id":"47c8dbff2af371bcefa6f92bd8d9a458e90da2d2"}],"watcher/tests/conf/test_list_opts.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"edfb73fa157c4bba50371d72e3be8496ef867bc6","unresolved":false,"context_lines":[{"line_number":28,"context_line":"class TestListOpts(base.TestCase):"},{"line_number":29,"context_line":"    def setUp(self):"},{"line_number":30,"context_line":"        super(TestListOpts, self).setUp()"},{"line_number":31,"context_line":"        self.none_objects \u003d [\u0027DEFAULT\u0027, \u0027watcher_clients_auth\u0027,"},{"line_number":32,"context_line":"                             \u0027watcher_strategies.strategy_1\u0027]"},{"line_number":33,"context_line":"        self.base_sections \u003d ["},{"line_number":34,"context_line":"            \u0027DEFAULT\u0027, \u0027api\u0027, \u0027database\u0027, \u0027watcher_decision_engine\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_1c0b20d5","line":31,"updated":"2019-06-26 14:40:38.000000000","message":"It would be good to add a comment with this to explain it\u0027s a whitelist of option groups registered with a name rather than an OptGroup object, and that we should have options registered in list_opts using the OptGroup object over a string name.","commit_id":"d9a7a7274451856027d29978b11fa4dae80f8b2c"},{"author":{"_account_id":29911,"name":"Dantali0n","email":"info@dantalion.nl","username":"Dantali0n"},"change_message_id":"fd8f555d7c3d8b2fa5b93f0178a1ad164f46371e","unresolved":false,"context_lines":[{"line_number":28,"context_line":"class TestListOpts(base.TestCase):"},{"line_number":29,"context_line":"    def setUp(self):"},{"line_number":30,"context_line":"        super(TestListOpts, self).setUp()"},{"line_number":31,"context_line":"        self.none_objects \u003d [\u0027DEFAULT\u0027, \u0027watcher_clients_auth\u0027,"},{"line_number":32,"context_line":"                             \u0027watcher_strategies.strategy_1\u0027]"},{"line_number":33,"context_line":"        self.base_sections \u003d ["},{"line_number":34,"context_line":"            \u0027DEFAULT\u0027, \u0027api\u0027, \u0027database\u0027, \u0027watcher_decision_engine\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_3ff4d6f0","line":31,"in_reply_to":"9fb8cfa7_1c0b20d5","updated":"2019-06-26 15:15:50.000000000","message":"Done","commit_id":"d9a7a7274451856027d29978b11fa4dae80f8b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"edfb73fa157c4bba50371d72e3be8496ef867bc6","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def _assert_name_or_group(self, actual_sections, expected_sections):"},{"line_number":44,"context_line":"        for name_or_group, options in actual_sections:"},{"line_number":45,"context_line":"            section_name \u003d name_or_group"},{"line_number":46,"context_line":"        if isinstance(name_or_group, cfg.OptGroup):"},{"line_number":47,"context_line":"            section_name \u003d name_or_group.name"},{"line_number":48,"context_line":"        elif section_name in self.none_objects:"},{"line_number":49,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_bc4d343c","line":46,"updated":"2019-06-26 14:40:38.000000000","message":"I think you broke the nesting here - everything should be within the for loop correct?","commit_id":"d9a7a7274451856027d29978b11fa4dae80f8b2c"},{"author":{"_account_id":29911,"name":"Dantali0n","email":"info@dantalion.nl","username":"Dantali0n"},"change_message_id":"fd8f555d7c3d8b2fa5b93f0178a1ad164f46371e","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def _assert_name_or_group(self, actual_sections, expected_sections):"},{"line_number":44,"context_line":"        for name_or_group, options in actual_sections:"},{"line_number":45,"context_line":"            section_name \u003d name_or_group"},{"line_number":46,"context_line":"        if isinstance(name_or_group, cfg.OptGroup):"},{"line_number":47,"context_line":"            section_name \u003d name_or_group.name"},{"line_number":48,"context_line":"        elif section_name in self.none_objects:"},{"line_number":49,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_5cc75813","line":46,"in_reply_to":"9fb8cfa7_bc4d343c","updated":"2019-06-26 15:15:50.000000000","message":"Oops, you are correct. done","commit_id":"d9a7a7274451856027d29978b11fa4dae80f8b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"edfb73fa157c4bba50371d72e3be8496ef867bc6","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        elif section_name in self.none_objects:"},{"line_number":49,"context_line":"            pass"},{"line_number":50,"context_line":"        else:"},{"line_number":51,"context_line":"            raise Exception(\"Invalid option group: {0}\".format(section_name))"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        self.assertIn(section_name, expected_sections)"},{"line_number":54,"context_line":"        self.assertTrue(len(options))"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_bcfbf4bb","line":51,"updated":"2019-06-26 14:40:38.000000000","message":"Similarly a comment here could be useful if some new contributor is trying to add new options in a new group and fails this test - they might not understand what this means.","commit_id":"d9a7a7274451856027d29978b11fa4dae80f8b2c"},{"author":{"_account_id":29911,"name":"Dantali0n","email":"info@dantalion.nl","username":"Dantali0n"},"change_message_id":"fd8f555d7c3d8b2fa5b93f0178a1ad164f46371e","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        elif section_name in self.none_objects:"},{"line_number":49,"context_line":"            pass"},{"line_number":50,"context_line":"        else:"},{"line_number":51,"context_line":"            raise Exception(\"Invalid option group: {0}\".format(section_name))"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        self.assertIn(section_name, expected_sections)"},{"line_number":54,"context_line":"        self.assertTrue(len(options))"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_9fe2a22f","line":51,"in_reply_to":"9fb8cfa7_bcfbf4bb","updated":"2019-06-26 15:15:50.000000000","message":"Done","commit_id":"d9a7a7274451856027d29978b11fa4dae80f8b2c"},{"author":{"_account_id":28748,"name":"chenker","email":"chen.ke14@zte.com.cn","username":"chenke"},"change_message_id":"c43730cf58d79c073c77889fa2a640d2e8dcaa6d","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                # OptGroup object for some exceptions this is not possible but"},{"line_number":57,"context_line":"                # new groups should use OptGroup"},{"line_number":58,"context_line":"                raise Exception("},{"line_number":59,"context_line":"                    \"Invalid option group: {0} should be of type OptGroup not\""},{"line_number":60,"context_line":"                    \"string.\".format(section_name))"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        self.assertIn(section_name, expected_sections)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_bd0afcce","line":59,"range":{"start_line":59,"start_character":77,"end_line":59,"end_character":78},"updated":"2019-06-27 13:40:16.000000000","message":"space","commit_id":"aa738219cac43c9633f628cbf38e6f7a95958be0"},{"author":{"_account_id":29911,"name":"Dantali0n","email":"info@dantalion.nl","username":"Dantali0n"},"change_message_id":"9b96ae938cbf8cf5f8f35a1a9e243df58b501c13","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                # OptGroup object for some exceptions this is not possible but"},{"line_number":57,"context_line":"                # new groups should use OptGroup"},{"line_number":58,"context_line":"                raise Exception("},{"line_number":59,"context_line":"                    \"Invalid option group: {0} should be of type OptGroup not\""},{"line_number":60,"context_line":"                    \"string.\".format(section_name))"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        self.assertIn(section_name, expected_sections)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_d538c0a0","line":59,"range":{"start_line":59,"start_character":77,"end_line":59,"end_character":78},"in_reply_to":"9fb8cfa7_bd0afcce","updated":"2019-07-01 07:30:47.000000000","message":"Done","commit_id":"aa738219cac43c9633f628cbf38e6f7a95958be0"}]}
