)]}'
{"ironic/drivers/modules/pxe.py":[{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"8a3e70846bef93eeb9b6b4d6564de99b78cc0a0f","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        # Try to get the URL of the Ironic API"},{"line_number":549,"context_line":"        try:"},{"line_number":550,"context_line":"            CONF.conductor.api_url or keystone.get_service_url()"},{"line_number":551,"context_line":"        except (ksexception.Unauthorized, ksexception.AuthorizationFailure,"},{"line_number":552,"context_line":"                ksexception.EndpointNotFound, exception.CatalogFailure):"},{"line_number":553,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":554,"context_line":"                \"Couldn\u0027t get the URL of the Ironic API service from the \""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAVn%2F%2B6W4%3D","line":551,"updated":"2014-03-11 11:11:49.000000000","message":"keystone.get_service_url() converts keystoneclient exceptions to Ironic exceptions,\nksexception should not be here. And maybe add test that api url is valid?","commit_id":"c593d52f62374d81227dc594f97341eefd987538"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e07e47d79bc19fb59bf324730f0462bb653ec62c","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        # Try to get the URL of the Ironic API"},{"line_number":549,"context_line":"        try:"},{"line_number":550,"context_line":"            CONF.conductor.api_url or keystone.get_service_url()"},{"line_number":551,"context_line":"        except (ksexception.Unauthorized, ksexception.AuthorizationFailure,"},{"line_number":552,"context_line":"                ksexception.EndpointNotFound, exception.CatalogFailure):"},{"line_number":553,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":554,"context_line":"                \"Couldn\u0027t get the URL of the Ironic API service from the \""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAVn%2F%2B5%2Bs%3D","line":551,"in_reply_to":"AAAAVn%2F%2B6U8%3D","updated":"2014-03-11 12:16:20.000000000","message":"+1","commit_id":"c593d52f62374d81227dc594f97341eefd987538"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"1e7f571473ade492c75af7d62628c01f069cf483","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        # Try to get the URL of the Ironic API"},{"line_number":549,"context_line":"        try:"},{"line_number":550,"context_line":"            CONF.conductor.api_url or keystone.get_service_url()"},{"line_number":551,"context_line":"        except (ksexception.Unauthorized, ksexception.AuthorizationFailure,"},{"line_number":552,"context_line":"                ksexception.EndpointNotFound, exception.CatalogFailure):"},{"line_number":553,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":554,"context_line":"                \"Couldn\u0027t get the URL of the Ironic API service from the \""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAVn%2F%2B6U8%3D","line":551,"in_reply_to":"AAAAVn%2F%2B6W4%3D","updated":"2014-03-11 12:14:51.000000000","message":"Ops, indeed it\u0027s converting, will fix it.\n\nFor the validation I didn\u0027t find any method in oslo or ironic to validate the URL format, so I will leave a TODO there, this would be work for another patch","commit_id":"c593d52f62374d81227dc594f97341eefd987538"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"dccc74e390548d3ec5252668efd961559740281b","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        try:"},{"line_number":549,"context_line":"            # TODO(lucasagomes): Validate the format of the URL"},{"line_number":550,"context_line":"            CONF.conductor.api_url or keystone.get_service_url()"},{"line_number":551,"context_line":"        except exception.CatalogFailure:"},{"line_number":552,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":553,"context_line":"                \"Couldn\u0027t get the URL of the Ironic API service from the \""},{"line_number":554,"context_line":"                \"configuration file or keystone catalog.\"))"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVn%2F%2B5RA%3D","line":551,"updated":"2014-03-11 13:47:30.000000000","message":"Shouldn\u0027t we also catch CatalogUnauthorized and CatalogNotFound here?","commit_id":"3a4ea8ac8f167e2ed892d112b4131ffbc2ae5c31"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"2f4fe3026d5c1a8628f39f4fd6def55431dda6af","unresolved":false,"context_lines":[{"line_number":553,"context_line":"                exception.CatalogUnauthorized):"},{"line_number":554,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":555,"context_line":"                \"Couldn\u0027t get the URL of the Ironic API service from the \""},{"line_number":556,"context_line":"                \"configuration file or keystone catalog.\"))"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"    @task_manager.require_exclusive_lock"},{"line_number":559,"context_line":"    def deploy(self, task, node):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2Bzv4%3D","line":556,"updated":"2014-03-12 01:46:39.000000000","message":"would it be helpful to the user if we put eg \u0027(conductor.api_url)\u0027. Although it is documented in the sample config file.\n\nI don\u0027t know that it matters or not, but I\u0027ve seen it referred to as \u0027keystone service catalog\u0027.","commit_id":"49b6c8bf75139fc737afed49e1db28648ab08693"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"b105f8e928a473989e703199c1aae023f83abd1b","unresolved":false,"context_lines":[{"line_number":553,"context_line":"                exception.CatalogUnauthorized):"},{"line_number":554,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":555,"context_line":"                \"Couldn\u0027t get the URL of the Ironic API service from the \""},{"line_number":556,"context_line":"                \"configuration file or keystone catalog.\"))"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"    @task_manager.require_exclusive_lock"},{"line_number":559,"context_line":"    def deploy(self, task, node):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2F%2B3E%3D","line":556,"in_reply_to":"AAAAVn%2F%2Bwyc%3D","updated":"2014-03-12 15:07:21.000000000","message":"Makes sense Lucas. Thx!","commit_id":"49b6c8bf75139fc737afed49e1db28648ab08693"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8b817ecf3c05d5ed162355bfe65de7739f6f2a32","unresolved":false,"context_lines":[{"line_number":553,"context_line":"                exception.CatalogUnauthorized):"},{"line_number":554,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":555,"context_line":"                \"Couldn\u0027t get the URL of the Ironic API service from the \""},{"line_number":556,"context_line":"                \"configuration file or keystone catalog.\"))"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"    @task_manager.require_exclusive_lock"},{"line_number":559,"context_line":"    def deploy(self, task, node):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2Bwyc%3D","line":556,"in_reply_to":"AAAAVn%2F%2Bzv4%3D","updated":"2014-03-12 10:39:45.000000000","message":"Hey Ruby,\n\nI usually try to avoid adding the configuration option names to strings like that because in case someone changes the name there\u0027s a big change that it won\u0027t be changed in the strings and then what\u0027s been returned to the user will be wrong.\n\nSo I prefer to avoid it, the user can check out the user docs about the configurations and set it.","commit_id":"49b6c8bf75139fc737afed49e1db28648ab08693"}],"ironic/tests/drivers/test_pxe.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"33eb57ed5e81dec1892826341c501bea0d620493","unresolved":false,"context_lines":[{"line_number":652,"context_line":"        # not present in the keystone catalog"},{"line_number":653,"context_line":"        mock_ks.side_effect \u003d exception.CatalogFailure"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":"        with task_manager.acquire(self.context, [self.node[\u0027uuid\u0027]],"},{"line_number":656,"context_line":"                                  shared\u003dTrue) as task:"},{"line_number":657,"context_line":"            task.resources[0].driver.deploy.validate(task, self.node)"},{"line_number":658,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVn%2F%2B5Rk%3D","line":655,"updated":"2014-03-11 13:47:17.000000000","message":"I think it would be better if:\n    self.node.uuid","commit_id":"3a4ea8ac8f167e2ed892d112b4131ffbc2ae5c31"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"33eb57ed5e81dec1892826341c501bea0d620493","unresolved":false,"context_lines":[{"line_number":663,"context_line":"        # not present in the config file"},{"line_number":664,"context_line":"        self.config(group\u003d\u0027conductor\u0027, api_url\u003dNone)"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        with task_manager.acquire(self.context, [self.node[\u0027uuid\u0027]],"},{"line_number":667,"context_line":"                                  shared\u003dTrue) as task:"},{"line_number":668,"context_line":"            task.resources[0].driver.deploy.validate(task, self.node)"},{"line_number":669,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVn%2F%2B5Rc%3D","line":666,"updated":"2014-03-11 13:47:17.000000000","message":"ditto","commit_id":"3a4ea8ac8f167e2ed892d112b4131ffbc2ae5c31"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"33eb57ed5e81dec1892826341c501bea0d620493","unresolved":false,"context_lines":[{"line_number":674,"context_line":"        # not present in the config file"},{"line_number":675,"context_line":"        self.config(group\u003d\u0027conductor\u0027, api_url\u003dNone)"},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"        with task_manager.acquire(self.context, [self.node[\u0027uuid\u0027]],"},{"line_number":678,"context_line":"                                  shared\u003dTrue) as task:"},{"line_number":679,"context_line":"            self.assertRaises(exception.InvalidParameterValue,"},{"line_number":680,"context_line":"                              task.resources[0].driver.deploy.validate,"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVn%2F%2B5RQ%3D","line":677,"updated":"2014-03-11 13:47:17.000000000","message":"ditto","commit_id":"3a4ea8ac8f167e2ed892d112b4131ffbc2ae5c31"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"2f4fe3026d5c1a8628f39f4fd6def55431dda6af","unresolved":false,"context_lines":[{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        with task_manager.acquire(self.context, [self.node.uuid],"},{"line_number":667,"context_line":"                                  shared\u003dTrue) as task:"},{"line_number":668,"context_line":"            task.resources[0].driver.deploy.validate(task, self.node)"},{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    @mock.patch.object(keystone, \u0027get_service_url\u0027)"},{"line_number":671,"context_line":"    def test_validate_fail_no_api_url(self, mock_ks):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2Bzso%3D","line":668,"updated":"2014-03-12 01:46:39.000000000","message":"I don\u0027t know if it is worth doing or not, but what about checking mock_ks.assert_called_once_with(blah blah)","commit_id":"49b6c8bf75139fc737afed49e1db28648ab08693"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8b817ecf3c05d5ed162355bfe65de7739f6f2a32","unresolved":false,"context_lines":[{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        with task_manager.acquire(self.context, [self.node.uuid],"},{"line_number":667,"context_line":"                                  shared\u003dTrue) as task:"},{"line_number":668,"context_line":"            task.resources[0].driver.deploy.validate(task, self.node)"},{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    @mock.patch.object(keystone, \u0027get_service_url\u0027)"},{"line_number":671,"context_line":"    def test_validate_fail_no_api_url(self, mock_ks):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2BwyQ%3D","line":668,"in_reply_to":"AAAAVn%2F%2Bzso%3D","updated":"2014-03-12 10:39:45.000000000","message":"Will do :)","commit_id":"49b6c8bf75139fc737afed49e1db28648ab08693"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"2f4fe3026d5c1a8628f39f4fd6def55431dda6af","unresolved":false,"context_lines":[{"line_number":678,"context_line":"                                  shared\u003dTrue) as task:"},{"line_number":679,"context_line":"            self.assertRaises(exception.InvalidParameterValue,"},{"line_number":680,"context_line":"                              task.resources[0].driver.deploy.validate,"},{"line_number":681,"context_line":"                              task, self.node)"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"    def test__get_nodes_mac_addresses(self):"},{"line_number":684,"context_line":"        ports \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2Bzr4%3D","line":681,"updated":"2014-03-12 01:46:39.000000000","message":"Here I would check for mock_ks.assert_called_once_with... (in case some other part of the .validate() raises this exception).","commit_id":"49b6c8bf75139fc737afed49e1db28648ab08693"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8b817ecf3c05d5ed162355bfe65de7739f6f2a32","unresolved":false,"context_lines":[{"line_number":678,"context_line":"                                  shared\u003dTrue) as task:"},{"line_number":679,"context_line":"            self.assertRaises(exception.InvalidParameterValue,"},{"line_number":680,"context_line":"                              task.resources[0].driver.deploy.validate,"},{"line_number":681,"context_line":"                              task, self.node)"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"    def test__get_nodes_mac_addresses(self):"},{"line_number":684,"context_line":"        ports \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2BwyM%3D","line":681,"in_reply_to":"AAAAVn%2F%2Bzr4%3D","updated":"2014-03-12 10:39:45.000000000","message":"Will do :)","commit_id":"49b6c8bf75139fc737afed49e1db28648ab08693"}]}