)]}'
{"config_tempest/services/baremetal.py":[{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"1b0b617c68338418b467b263c6574cf47fab3394","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def post_configuration(self, conf, is_service):"},{"line_number":22,"context_line":"        if conf.get(\u0027service_available\u0027, \u0027ironic\u0027):"},{"line_number":23,"context_line":"            if not conf.has_option(\u0027auth\u0027, \u0027create_isolated_networks\u0027):"},{"line_number":24,"context_line":"                conf.set(\u0027auth\u0027, \u0027create_isolated_networks\u0027, \u0027False\u0027)"},{"line_number":25,"context_line":"            else:"},{"line_number":26,"context_line":"                if conf.get(\u0027auth\u0027, \u0027create_isolated_networks\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_9d02bc08","line":23,"updated":"2020-05-13 09:56:27.000000000","message":"The result of this block of if... else is that auth.create_isolated_networks is always set to \u0027false\u0027.\n(if not set -\u003e set it to False; if set and True, set it to False).\n\nCouldn\u0027t it be replaced by a simple conf.set.... line?\n\nAlso: wouldn\u0027t that break user overrides?","commit_id":"d362a8cb693ae0c0b73cfc5095984ac6cc200312"},{"author":{"_account_id":30750,"name":"amolkahat","display_name":"Amol Kahat","email":"amolkahat@gmail.com","username":"amolkahat"},"change_message_id":"bc49b509af6aae064f84a931374a2cfa91a06f27","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def post_configuration(self, conf, is_service):"},{"line_number":22,"context_line":"        if conf.get(\u0027service_available\u0027, \u0027ironic\u0027):"},{"line_number":23,"context_line":"            if not conf.has_option(\u0027auth\u0027, \u0027create_isolated_networks\u0027):"},{"line_number":24,"context_line":"                conf.set(\u0027auth\u0027, \u0027create_isolated_networks\u0027, \u0027False\u0027)"},{"line_number":25,"context_line":"            else:"},{"line_number":26,"context_line":"                if conf.get(\u0027auth\u0027, \u0027create_isolated_networks\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_29f4e4ce","line":23,"in_reply_to":"ff570b3c_2fd512ba","updated":"2020-05-15 05:54:59.000000000","message":"\u003e I agree with tosky, i don\u0027t see a reason why the condition is\n \u003e needed ...\n \u003e \n \u003e btw, it wouldn\u0027t break overrides as they are set with priority -\n \u003e can be overridden only with the set command with priority set to\n \u003e True.\n\n\nMy first concern is not to break overrides. That\u0027s why those conditions are here. As you said overridden set with \"priority\u003dTrue\", then it makes seance. I\u0027ll remove conditions. :)","commit_id":"d362a8cb693ae0c0b73cfc5095984ac6cc200312"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"190a19fe24adfbcada46f57537419d77184f36ae","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def post_configuration(self, conf, is_service):"},{"line_number":22,"context_line":"        if conf.get(\u0027service_available\u0027, \u0027ironic\u0027):"},{"line_number":23,"context_line":"            if not conf.has_option(\u0027auth\u0027, \u0027create_isolated_networks\u0027):"},{"line_number":24,"context_line":"                conf.set(\u0027auth\u0027, \u0027create_isolated_networks\u0027, \u0027False\u0027)"},{"line_number":25,"context_line":"            else:"},{"line_number":26,"context_line":"                if conf.get(\u0027auth\u0027, \u0027create_isolated_networks\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_2fd512ba","line":23,"in_reply_to":"ff570b3c_9d02bc08","updated":"2020-05-14 16:00:35.000000000","message":"I agree with tosky, i don\u0027t see a reason why the condition is needed ...\n\nbtw, it wouldn\u0027t break overrides as they are set with priority - can be overridden only with the set command with priority set to True.","commit_id":"d362a8cb693ae0c0b73cfc5095984ac6cc200312"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"1b0b617c68338418b467b263c6574cf47fab3394","unresolved":false,"context_lines":[{"line_number":26,"context_line":"                if conf.get(\u0027auth\u0027, \u0027create_isolated_networks\u0027):"},{"line_number":27,"context_line":"                    conf.set(\u0027auth\u0027, \u0027create_isolated_networks\u0027, \u0027False\u0027)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"            if not conf.has_option(\u0027validation\u0027, \u0027connect_method\u0027):"},{"line_number":30,"context_line":"                conf.set(\u0027validation\u0027, \u0027connect_method\u0027, \u0027fixed\u0027)"},{"line_number":31,"context_line":"            else:"},{"line_number":32,"context_line":"                if conf.get(\u0027validation\u0027, \u0027connect_method\u0027) !\u003d \"fixed\":"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_fd09382c","line":29,"updated":"2020-05-13 09:56:27.000000000","message":"See above","commit_id":"d362a8cb693ae0c0b73cfc5095984ac6cc200312"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"1b0b617c68338418b467b263c6574cf47fab3394","unresolved":false,"context_lines":[{"line_number":32,"context_line":"                if conf.get(\u0027validation\u0027, \u0027connect_method\u0027) !\u003d \"fixed\":"},{"line_number":33,"context_line":"                    conf.set(\u0027validation\u0027, \u0027connect_method\u0027, \u0027fixed\u0027)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"            if not conf.has_option(\u0027validation\u0027, \u0027network_for_ssh\u0027):"},{"line_number":36,"context_line":"                conf.set(\u0027validation\u0027, \u0027network_for_ssh\u0027, \u0027provisioning\u0027)"},{"line_number":37,"context_line":"            else:"},{"line_number":38,"context_line":"                if conf.get(\u0027validation\u0027, \u0027network_for_ssh\u0027) !\u003d \u0027provisioning\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_dd0c343d","line":35,"updated":"2020-05-13 09:56:27.000000000","message":"See above","commit_id":"d362a8cb693ae0c0b73cfc5095984ac6cc200312"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"1b0b617c68338418b467b263c6574cf47fab3394","unresolved":false,"context_lines":[{"line_number":38,"context_line":"                if conf.get(\u0027validation\u0027, \u0027network_for_ssh\u0027) !\u003d \u0027provisioning\u0027:"},{"line_number":39,"context_line":"                    conf.set(\u0027validation\u0027, \u0027network_for_ssh\u0027, \u0027provisioning\u0027)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"            if not conf.has_option(\u0027baremetal\u0027, \u0027use_provision_network\u0027):"},{"line_number":42,"context_line":"                conf.set(\u0027baremetal\u0027, \u0027use_provision_network\u0027, \u0027True\u0027)"},{"line_number":43,"context_line":"            else:"},{"line_number":44,"context_line":"                if not conf.get(\u0027baremetal\u0027, \u0027use_provision_network\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff570b3c_3d13b05e","line":41,"updated":"2020-05-13 09:56:27.000000000","message":"See above","commit_id":"d362a8cb693ae0c0b73cfc5095984ac6cc200312"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"190a19fe24adfbcada46f57537419d77184f36ae","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def post_configuration(self, conf, is_service):"},{"line_number":22,"context_line":"        if conf.get(\u0027service_available\u0027, \u0027ironic\u0027):"},{"line_number":23,"context_line":"            if conf.has_option(\u0027auth\u0027, \u0027create_isolated_networks\u0027):"},{"line_number":24,"context_line":"                conf.set(\u0027auth\u0027, \u0027create_isolated_networks\u0027, \u0027False\u0027)"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"            if conf.has_option(\u0027validation\u0027, \u0027connect_method\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff570b3c_2f87d2d9","line":23,"range":{"start_line":23,"start_character":0,"end_line":23,"end_character":67},"updated":"2020-05-14 16:00:35.000000000","message":"is this condition and actually all the ones below correct? the default value in tempest is True: https://opendev.org/openstack/tempest/src/branch/master/tempest/config.py#L73\n\ndo we really need those conditions (conf.has_option ...), why?","commit_id":"bc171a753b2573a4d9007916dc7de07d2882bca9"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"e28c571b3a510f8b6a4b237db209d272451ee473","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class BaremetalService(Service):"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def post_configuration(self, conf, is_service):"},{"line_number":22,"context_line":"        if conf.get(\u0027service_available\u0027, \u0027ironic\u0027):"},{"line_number":23,"context_line":"            conf.set(\u0027auth\u0027, \u0027create_isolated_networks\u0027, \u0027False\u0027)"},{"line_number":24,"context_line":"            conf.set(\u0027validation\u0027, \u0027connect_method\u0027, \u0027fixed\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff570b3c_8457b158","line":21,"range":{"start_line":21,"start_character":8,"end_line":21,"end_character":26},"updated":"2020-05-18 08:53:48.000000000","message":"instead of post_configuration, please use set_default_tempest_options method [1] - that way you don\u0027t need to have if condition because set_default_tempest_options is called only when the service is discovered [2]\n\n[1] https://opendev.org/openstack/python-tempestconf/src/commit/ad9304b7943c8401bd2f488c7d01e089c1d136f3/config_tempest/services/services.py#L122\n[2] https://opendev.org/openstack/python-tempestconf/src/commit/ad9304b7943c8401bd2f488c7d01e089c1d136f3/config_tempest/services/services.py#L122","commit_id":"cdb7fd742115557f779bed47e119412ee70550e3"},{"author":{"_account_id":8367,"name":"Arx Cruz","email":"arxcruz@redhat.com","username":"arxcruz"},"change_message_id":"b9916683ec728fdc6e8b1f7225deb47372e15309","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def post_configuration(self, conf, is_service):"},{"line_number":22,"context_line":"        if conf.get(\u0027service_available\u0027, \u0027ironic\u0027):"},{"line_number":23,"context_line":"            conf.set(\u0027auth\u0027, \u0027create_isolated_networks\u0027, \u0027False\u0027)"},{"line_number":24,"context_line":"            conf.set(\u0027validation\u0027, \u0027connect_method\u0027, \u0027fixed\u0027)"},{"line_number":25,"context_line":"            conf.set(\u0027validation\u0027, \u0027network_for_ssh\u0027, \u0027provisioning\u0027)"},{"line_number":26,"context_line":"            conf.set(\u0027baremetal\u0027, \u0027use_provision_network\u0027, \u0027True\u0027)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff570b3c_876eb792","line":25,"range":{"start_line":23,"start_character":0,"end_line":25,"end_character":69},"updated":"2020-05-15 08:21:11.000000000","message":"my concern here is that these are not specific configuration to ironic, would it break other tests?","commit_id":"cdb7fd742115557f779bed47e119412ee70550e3"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"60504721c9c1db1a3bbfe3631e8f6efa3485ff97","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def post_configuration(self, conf, is_service):"},{"line_number":22,"context_line":"        if conf.get(\u0027service_available\u0027, \u0027ironic\u0027):"},{"line_number":23,"context_line":"            conf.set(\u0027auth\u0027, \u0027create_isolated_networks\u0027, \u0027False\u0027)"},{"line_number":24,"context_line":"            conf.set(\u0027validation\u0027, \u0027connect_method\u0027, \u0027fixed\u0027)"},{"line_number":25,"context_line":"            conf.set(\u0027validation\u0027, \u0027network_for_ssh\u0027, \u0027provisioning\u0027)"},{"line_number":26,"context_line":"            conf.set(\u0027baremetal\u0027, \u0027use_provision_network\u0027, \u0027True\u0027)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff570b3c_c229a19b","line":25,"range":{"start_line":23,"start_character":0,"end_line":25,"end_character":69},"in_reply_to":"ff570b3c_18c7a847","updated":"2020-05-17 16:06:40.000000000","message":"What Arx meant is that not all the parameters are ironic specific in a way they belong to ironic specific section (ironic, ironic_inspector or baremetal) - the parameters set here set values under auth and validation sections which may affect other tests","commit_id":"cdb7fd742115557f779bed47e119412ee70550e3"},{"author":{"_account_id":30750,"name":"amolkahat","display_name":"Amol Kahat","email":"amolkahat@gmail.com","username":"amolkahat"},"change_message_id":"9bcec85ccdf3ada06fccae56141c0b6e9096dcd2","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def post_configuration(self, conf, is_service):"},{"line_number":22,"context_line":"        if conf.get(\u0027service_available\u0027, \u0027ironic\u0027):"},{"line_number":23,"context_line":"            conf.set(\u0027auth\u0027, \u0027create_isolated_networks\u0027, \u0027False\u0027)"},{"line_number":24,"context_line":"            conf.set(\u0027validation\u0027, \u0027connect_method\u0027, \u0027fixed\u0027)"},{"line_number":25,"context_line":"            conf.set(\u0027validation\u0027, \u0027network_for_ssh\u0027, \u0027provisioning\u0027)"},{"line_number":26,"context_line":"            conf.set(\u0027baremetal\u0027, \u0027use_provision_network\u0027, \u0027True\u0027)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff570b3c_18c7a847","line":25,"range":{"start_line":23,"start_character":0,"end_line":25,"end_character":69},"in_reply_to":"ff570b3c_876eb792","updated":"2020-05-15 11:12:27.000000000","message":"Those parameters are specific to ironic. It will get added if ironic is enabled. I think it will not break the other tests.","commit_id":"cdb7fd742115557f779bed47e119412ee70550e3"}]}