)]}'
{"neutron/api/extensions.py":[{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"e76d134825f5212e4f2b3d3f32bac1f56e047a24","unresolved":false,"context_lines":[{"line_number":639,"context_line":"# Returns the extension paths from a config entry and the __path__"},{"line_number":640,"context_line":"# of neutron.extensions"},{"line_number":641,"context_line":"def get_extensions_path(service_plugins\u003dNone):"},{"line_number":642,"context_line":"    paths \u003d [neutron.extensions.__path__[0]]"},{"line_number":643,"context_line":"    if service_plugins:"},{"line_number":644,"context_line":"        for plugin in service_plugins.values():"},{"line_number":645,"context_line":"            neutron_mod \u003d provider_configuration.NeutronModule("}],"source_content_type":"text/x-python","patch_set":15,"id":"1a4dcd0f_004928d0","line":642,"updated":"2015-08-16 23:30:25.000000000","message":"wouldn\u0027t it be better to start this off as an OrderedDict?  That way you would preserve the addition order and not have to check for the existence of an item before appending.  Would allow removing the conversion list to OrderedDict later in the code as well (a piece you did not need to touch).","commit_id":"bfd5c38340b0ae8eaf098030dcb7bf9e786c6640"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"bb0d2abf5672c1961e569d0ceb87aeb63289ba18","unresolved":false,"context_lines":[{"line_number":639,"context_line":"# Returns the extension paths from a config entry and the __path__"},{"line_number":640,"context_line":"# of neutron.extensions"},{"line_number":641,"context_line":"def get_extensions_path(service_plugins\u003dNone):"},{"line_number":642,"context_line":"    paths \u003d [neutron.extensions.__path__[0]]"},{"line_number":643,"context_line":"    if service_plugins:"},{"line_number":644,"context_line":"        for plugin in service_plugins.values():"},{"line_number":645,"context_line":"            neutron_mod \u003d provider_configuration.NeutronModule("}],"source_content_type":"text/x-python","patch_set":15,"id":"fa1b9901_e6a3434e","line":642,"in_reply_to":"1a4dcd0f_004928d0","updated":"2015-08-17 15:32:58.000000000","message":"but then on line 654 I need to convert cfg.CONF.api_extensions_path to a dictionary in order to update the initial \u0027paths\u0027, or enumerate to add the elements. Let me see if it looks any cleaner.","commit_id":"bfd5c38340b0ae8eaf098030dcb7bf9e786c6640"}],"neutron/db/servicetype_db.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"608d068f97a3eae9c40d2852ede0dffb95248b93","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        return cls._instance"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def __init__(self):"},{"line_number":47,"context_line":"        self.config \u003d {}"},{"line_number":48,"context_line":"        # TODO(armax): remove these as soon as *-aaS start using"},{"line_number":49,"context_line":"        # the newly introduced add_provider_configuration API"},{"line_number":50,"context_line":"        self.config[\u0027LOADBALANCER\u0027] \u003d ("}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_729e47ba","line":47,"updated":"2015-08-07 03:59:53.000000000","message":"Nit: Why not init all the values as a dict literal?\n\nself.config \u003d {\n    \u0027LOADBALANCER\u0027: pconf.ProviderConfiguration(\u0027neutron_lbaas\u0027),\n    \u0027LOADBALANCERV2\u0027: pconf.ProviderConfiguration(\u0027neutron_lbaas\u0027),\n    \u0027FIREWALL\u0027: pconf.ProviderConfiguration(\u0027neutron_fwaas\u0027),\n    \u0027VPN\u0027: pconf.ProviderConfiguration(\u0027neutron_vpnaas\u0027)\n}","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"566bd2344523cce8ad4db5b217fe62f2842fbb2d","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        return cls._instance"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def __init__(self):"},{"line_number":47,"context_line":"        self.config \u003d {}"},{"line_number":48,"context_line":"        # TODO(armax): remove these as soon as *-aaS start using"},{"line_number":49,"context_line":"        # the newly introduced add_provider_configuration API"},{"line_number":50,"context_line":"        self.config[\u0027LOADBALANCER\u0027] \u003d ("}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_36df33d7","line":47,"in_reply_to":"1a4dcd0f_0da24699","updated":"2015-08-07 16:15:21.000000000","message":"ack","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"c40219bba02540aed5409de7ddffa955b9dc3747","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        return cls._instance"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def __init__(self):"},{"line_number":47,"context_line":"        self.config \u003d {}"},{"line_number":48,"context_line":"        # TODO(armax): remove these as soon as *-aaS start using"},{"line_number":49,"context_line":"        # the newly introduced add_provider_configuration API"},{"line_number":50,"context_line":"        self.config[\u0027LOADBALANCER\u0027] \u003d ("}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_0da24699","line":47,"in_reply_to":"1a4dcd0f_729e47ba","updated":"2015-08-07 04:43:56.000000000","message":"because the lines below are going away.","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"608d068f97a3eae9c40d2852ede0dffb95248b93","unresolved":false,"context_lines":[{"line_number":70,"context_line":"                        if svc_type in self.config)"},{"line_number":71,"context_line":"            )"},{"line_number":72,"context_line":"        return list("},{"line_number":73,"context_line":"            chain.from_iterable("},{"line_number":74,"context_line":"                self.config[p].get_service_providers(filters, fields)"},{"line_number":75,"context_line":"                for p in self.config)"},{"line_number":76,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_124a6326","line":73,"updated":"2015-08-07 03:59:53.000000000","message":"Why is this lazy eval necessary? (Here and line 67.)","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"c40219bba02540aed5409de7ddffa955b9dc3747","unresolved":false,"context_lines":[{"line_number":70,"context_line":"                        if svc_type in self.config)"},{"line_number":71,"context_line":"            )"},{"line_number":72,"context_line":"        return list("},{"line_number":73,"context_line":"            chain.from_iterable("},{"line_number":74,"context_line":"                self.config[p].get_service_providers(filters, fields)"},{"line_number":75,"context_line":"                for p in self.config)"},{"line_number":76,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_6db98acb","line":73,"in_reply_to":"1a4dcd0f_124a6326","updated":"2015-08-07 04:43:56.000000000","message":"I am building a list out of a list both here and there.","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"566bd2344523cce8ad4db5b217fe62f2842fbb2d","unresolved":false,"context_lines":[{"line_number":70,"context_line":"                        if svc_type in self.config)"},{"line_number":71,"context_line":"            )"},{"line_number":72,"context_line":"        return list("},{"line_number":73,"context_line":"            chain.from_iterable("},{"line_number":74,"context_line":"                self.config[p].get_service_providers(filters, fields)"},{"line_number":75,"context_line":"                for p in self.config)"},{"line_number":76,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_b1210da0","line":73,"in_reply_to":"1a4dcd0f_6db98acb","updated":"2015-08-07 16:15:21.000000000","message":"OK, got it. I am stupid. I had to run it in the debugger to fully understand what is going on here.","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"e76d134825f5212e4f2b3d3f32bac1f56e047a24","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        self.config \u003d {}"},{"line_number":48,"context_line":"        # TODO(armax): remove these as soon as *-aaS start using"},{"line_number":49,"context_line":"        # the newly introduced add_provider_configuration API"},{"line_number":50,"context_line":"        self.config[\u0027LOADBALANCER\u0027] \u003d ("},{"line_number":51,"context_line":"            pconf.ProviderConfiguration(\u0027neutron_lbaas\u0027))"},{"line_number":52,"context_line":"        self.config[\u0027LOADBALANCERV2\u0027] \u003d ("},{"line_number":53,"context_line":"            pconf.ProviderConfiguration(\u0027neutron_lbaas\u0027))"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a4dcd0f_a00e348a","line":50,"updated":"2015-08-16 23:30:25.000000000","message":"there are constants for these keys in neutron.plugins.common.constants. However, you might be trying to get rid of those constants too, but i dont see that in this review so I could be wrong.","commit_id":"bfd5c38340b0ae8eaf098030dcb7bf9e786c6640"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"bb0d2abf5672c1961e569d0ceb87aeb63289ba18","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        self.config \u003d {}"},{"line_number":48,"context_line":"        # TODO(armax): remove these as soon as *-aaS start using"},{"line_number":49,"context_line":"        # the newly introduced add_provider_configuration API"},{"line_number":50,"context_line":"        self.config[\u0027LOADBALANCER\u0027] \u003d ("},{"line_number":51,"context_line":"            pconf.ProviderConfiguration(\u0027neutron_lbaas\u0027))"},{"line_number":52,"context_line":"        self.config[\u0027LOADBALANCERV2\u0027] \u003d ("},{"line_number":53,"context_line":"            pconf.ProviderConfiguration(\u0027neutron_lbaas\u0027))"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa1b9901_c6b39f09","line":50,"in_reply_to":"1a4dcd0f_a00e348a","updated":"2015-08-17 15:32:58.000000000","message":"it\u0027s in the dependent patch.","commit_id":"bfd5c38340b0ae8eaf098030dcb7bf9e786c6640"}],"neutron/services/provider_configuration.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"608d068f97a3eae9c40d2852ede0dffb95248b93","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            \u0027ini\u0027: None"},{"line_number":49,"context_line":"        }"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _import_or_none(self):"},{"line_number":52,"context_line":"            try:"},{"line_number":53,"context_line":"                return importlib.import_module(self.module_name)"},{"line_number":54,"context_line":"            except ImportError:"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_72e087cd","line":51,"updated":"2015-08-07 03:59:53.000000000","message":"Nit: This is the same as oslo_utils.importutils.try_import(). You could use that instead of importlib.","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"566bd2344523cce8ad4db5b217fe62f2842fbb2d","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            \u0027ini\u0027: None"},{"line_number":49,"context_line":"        }"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _import_or_none(self):"},{"line_number":52,"context_line":"            try:"},{"line_number":53,"context_line":"                return importlib.import_module(self.module_name)"},{"line_number":54,"context_line":"            except ImportError:"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_51444948","line":51,"in_reply_to":"1a4dcd0f_0dc9e658","updated":"2015-08-07 16:15:21.000000000","message":"It is a nit. I wouldn\u0027t bother.","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"c40219bba02540aed5409de7ddffa955b9dc3747","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            \u0027ini\u0027: None"},{"line_number":49,"context_line":"        }"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _import_or_none(self):"},{"line_number":52,"context_line":"            try:"},{"line_number":53,"context_line":"                return importlib.import_module(self.module_name)"},{"line_number":54,"context_line":"            except ImportError:"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_0dc9e658","line":51,"in_reply_to":"1a4dcd0f_72e087cd","updated":"2015-08-07 04:43:56.000000000","message":"This is a straight move, I can take of this in a follow-up?","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"},{"author":{"_account_id":2888,"name":"Mathieu Rohon","email":"mathieu.rohon@gmail.com","username":"mathieu-rohon"},"change_message_id":"e9acd683c8f62a74a6bff39d668ad248ce40a7a8","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"class ProviderConfiguration(object):"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def __init__(self, svc_module\u003d\u0027neutron\u0027):"},{"line_number":188,"context_line":"        self.providers \u003d {}"},{"line_number":189,"context_line":"        for prov in parse_service_provider_opt(svc_module):"},{"line_number":190,"context_line":"            self.add_provider(prov)"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_0a5a502a","line":187,"updated":"2015-08-06 12:45:13.000000000","message":"with this patch, it seems that we are loosing the capacity to give the provider configuration file a different name than the module name. \n\nOn networking-bgpvpn for instance, without this patch, the config file could be named neutron_bgpvpn.conf. But now it has to be named networking-bgpvpn.conf, since the conf file is guessed with the name of the module.\n\nsee https://review.openstack.org/#/c/208527/3","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"971f4dd02df99e37c5c20fa4a3c169bb721c75ff","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"class ProviderConfiguration(object):"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def __init__(self, svc_module\u003d\u0027neutron\u0027):"},{"line_number":188,"context_line":"        self.providers \u003d {}"},{"line_number":189,"context_line":"        for prov in parse_service_provider_opt(svc_module):"},{"line_number":190,"context_line":"            self.add_provider(prov)"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_d7bde402","line":187,"in_reply_to":"1a4dcd0f_0a5a502a","updated":"2015-08-06 16:08:40.000000000","message":"That\u0027s true, but do you know of *any* OpenStack project where the config file is named differently from the project itself? This sounds purely cosmetic, that said, if we wanted to overcome this limitation, it wouldn\u0027t be too hard to specify both module name and config file.","commit_id":"ed158445a020fee46b08f5570e2ae27c5b07ffcd"}]}