)]}'
{"ironic/api/controllers/v1/node.py":[{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"52729210ee5aefa2d2a484e54013dd492f41867f","unresolved":false,"context_lines":[{"line_number":272,"context_line":"        for k in self.fields:"},{"line_number":273,"context_line":"            setattr(self, k, kwargs.get(k))"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        # NOTE(lucasagomes): chassis_uuid is not part of objects.Port.fields"},{"line_number":276,"context_line":"        #                    because it\u0027s an API-only attribute"},{"line_number":277,"context_line":"        self.fields.append(\u0027chassis_uuid\u0027)"},{"line_number":278,"context_line":"        setattr(self, \u0027chassis_uuid\u0027, kwargs.get(\u0027chassis_id\u0027, None))"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9Z1U%3D","line":275,"updated":"2013-12-11 21:50:15.000000000","message":"Do you mean objects.Node.fields?","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ba99d8f937e1343cb4867d7145e8680beb861c88","unresolved":false,"context_lines":[{"line_number":272,"context_line":"        for k in self.fields:"},{"line_number":273,"context_line":"            setattr(self, k, kwargs.get(k))"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        # NOTE(lucasagomes): chassis_uuid is not part of objects.Port.fields"},{"line_number":276,"context_line":"        #                    because it\u0027s an API-only attribute"},{"line_number":277,"context_line":"        self.fields.append(\u0027chassis_uuid\u0027)"},{"line_number":278,"context_line":"        setattr(self, \u0027chassis_uuid\u0027, kwargs.get(\u0027chassis_id\u0027, None))"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9UyY%3D","line":275,"in_reply_to":"AAAATn%2F9Z1U%3D","updated":"2013-12-12 17:19:33.000000000","message":"heh yes! Thanks will fix.","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"52729210ee5aefa2d2a484e54013dd492f41867f","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                    \"can only be true or false.\") % associated)"},{"line_number":425,"context_line":"        return nodes"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"    def _ensure_driver_is_supported(self, driver):"},{"line_number":428,"context_line":"        drivers \u003d pecan.request.dbapi.list_active_conductor_drivers("},{"line_number":429,"context_line":"                                     interval\u003dCONF.conductor.heartbeat_timeout)"},{"line_number":430,"context_line":"        if driver not in drivers:"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9Z04%3D","line":427,"updated":"2013-12-11 21:50:15.000000000","message":"Nice touch!\n\nExcept that this change isn\u0027t mentioned in the commit message, and it\u0027s a new feature that doesn\u0027t seem related to the rest of this change. It would be great as a separate patch... but at a minimum, needs to be laid out in the commit message.","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"71d910b98766a327b9212b8021f3be9161ced8d2","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                    \"can only be true or false.\") % associated)"},{"line_number":425,"context_line":"        return nodes"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"    def _ensure_driver_is_supported(self, driver):"},{"line_number":428,"context_line":"        drivers \u003d pecan.request.dbapi.list_active_conductor_drivers("},{"line_number":429,"context_line":"                                     interval\u003dCONF.conductor.heartbeat_timeout)"},{"line_number":430,"context_line":"        if driver not in drivers:"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9Q%2FE%3D","line":427,"in_reply_to":"AAAATn%2F9UxA%3D","updated":"2013-12-13 10:31:16.000000000","message":"Agree :) As for bug 1238167, I think we can close it after this fix landed, because all the points mentioned there will be done.","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ba99d8f937e1343cb4867d7145e8680beb861c88","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                    \"can only be true or false.\") % associated)"},{"line_number":425,"context_line":"        return nodes"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"    def _ensure_driver_is_supported(self, driver):"},{"line_number":428,"context_line":"        drivers \u003d pecan.request.dbapi.list_active_conductor_drivers("},{"line_number":429,"context_line":"                                     interval\u003dCONF.conductor.heartbeat_timeout)"},{"line_number":430,"context_line":"        if driver not in drivers:"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9UxA%3D","line":427,"in_reply_to":"AAAATn%2F9WE8%3D","updated":"2013-12-12 17:19:33.000000000","message":"Sorry @Max I didn\u0027t want to step on ur toes, when I was doing it sounded like a good check for the driver attribute, just like checking if the uuid on the chassis_uuid is valid as well.\n\nAnd I agree that it should be part of another patch, this patch is already closing one bug and partially fixing another (Closes-Bug: #1252213, Partial-Bug: #1223847) so that really looks like something that needs to be splitted.\n\nI will create another patch for it and link the bug on the commit message.\n\nThank you guys for the review and suggestions","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ea36f6b7ebe519611682bc904baa2e44be40ab4c","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                    \"can only be true or false.\") % associated)"},{"line_number":425,"context_line":"        return nodes"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"    def _ensure_driver_is_supported(self, driver):"},{"line_number":428,"context_line":"        drivers \u003d pecan.request.dbapi.list_active_conductor_drivers("},{"line_number":429,"context_line":"                                     interval\u003dCONF.conductor.heartbeat_timeout)"},{"line_number":430,"context_line":"        if driver not in drivers:"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9WE8%3D","line":427,"in_reply_to":"AAAATn%2F9Z04%3D","updated":"2013-12-12 16:20:04.000000000","message":"Also this seems to going to fix the bug 1238167 which I wanted to start \u003d) Anyway I\u0027m fine if this will be done here, just mention this in commit message and please add ref to the bug.","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ea36f6b7ebe519611682bc904baa2e44be40ab4c","unresolved":false,"context_lines":[{"line_number":481,"context_line":"        if self._from_chassis:"},{"line_number":482,"context_line":"            raise exception.OperationNotPermitted"},{"line_number":483,"context_line":""},{"line_number":484,"context_line":"        self._ensure_driver_is_supported(node.driver)"},{"line_number":485,"context_line":"        try:"},{"line_number":486,"context_line":"            new_node \u003d pecan.request.dbapi.create_node(node.as_dict())"},{"line_number":487,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9VW4%3D","line":484,"updated":"2013-12-12 16:20:04.000000000","message":"I\u0027m wondering if this enough to make sure that all our nodes have valid drivers. Imagine at this moment we have a conductor with such driver, but then it goes offline and node becomes invalid. \n\nFor this patch it\u0027s definitely OK, but do we have some plan for the future? Maybe during the periodic task when touching conductors also make sure that we don\u0027t have invalid nodes. And if the node is invalid then mark it, so when it accessed the next time we\u0027ll show \"Node driver has become unavailable since the last request\" instead of current EAFP approach and \"Failed to load driver...\" message. How do you think is this reasonable idea?","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"71d910b98766a327b9212b8021f3be9161ced8d2","unresolved":false,"context_lines":[{"line_number":481,"context_line":"        if self._from_chassis:"},{"line_number":482,"context_line":"            raise exception.OperationNotPermitted"},{"line_number":483,"context_line":""},{"line_number":484,"context_line":"        self._ensure_driver_is_supported(node.driver)"},{"line_number":485,"context_line":"        try:"},{"line_number":486,"context_line":"            new_node \u003d pecan.request.dbapi.create_node(node.as_dict())"},{"line_number":487,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9Q9Y%3D","line":484,"in_reply_to":"AAAATn%2F9UuU%3D","updated":"2013-12-13 10:31:16.000000000","message":"Thank you for the explanation. I\u0027m totally agree that it\u0027s all we can do on the API side, and you provided a good plan for the remaining work :)","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ba99d8f937e1343cb4867d7145e8680beb861c88","unresolved":false,"context_lines":[{"line_number":481,"context_line":"        if self._from_chassis:"},{"line_number":482,"context_line":"            raise exception.OperationNotPermitted"},{"line_number":483,"context_line":""},{"line_number":484,"context_line":"        self._ensure_driver_is_supported(node.driver)"},{"line_number":485,"context_line":"        try:"},{"line_number":486,"context_line":"            new_node \u003d pecan.request.dbapi.create_node(node.as_dict())"},{"line_number":487,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9UuU%3D","line":484,"in_reply_to":"AAAATn%2F9VW4%3D","updated":"2013-12-12 17:19:33.000000000","message":"I think that\u0027s a very good point, on the API side I don\u0027t think there\u0027s much we can do apart from checking whether the driver is valid or not in the moment the node is created/updated.\n\nNow once we have the consistent hashing algorithm in place[1], which do this mapping between conductors to nodes (considering the driver the conductor have)  and need to maintain a list of dead conductors we might take some action there. Also we need to expose a way to mark a node as \u0027temporarily unavailable\u0027 and remove it from the scheduler[2] to avoid the scheduler from picking it to deploy something on it, so when we have this list and the ability to mark the node as unavailable I think we might have all the pieces to do this work you mentioned.\n\n[1] https://blueprints.launchpad.net/ironic/+spec/instance-mapping-by-consistent-hash\n\n[2] https://bugs.launchpad.net/ironic/+bug/1260099","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"84a31f36fb7b06700918ca5aa34bb2ea57c8b0fa","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        if rpc_node[\u0027target_power_state\u0027] or \\"},{"line_number":503,"context_line":"             rpc_node[\u0027target_provision_state\u0027]:"},{"line_number":504,"context_line":"            msg \u003d _(\u0027Node %s can not be updated while a state transition\u0027"},{"line_number":505,"context_line":"                     \u0027is in progress.\u0027)"},{"line_number":506,"context_line":"            raise wsme.exc.ClientSideError(msg % uuid, status_code\u003d409)"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9XrA%3D","line":505,"updated":"2013-12-12 07:10:13.000000000","message":"Lucas, a minor comment here - it is better we use double quotes for message string, and the single quotes is for identifier name.","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ba99d8f937e1343cb4867d7145e8680beb861c88","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        if rpc_node[\u0027target_power_state\u0027] or \\"},{"line_number":503,"context_line":"             rpc_node[\u0027target_provision_state\u0027]:"},{"line_number":504,"context_line":"            msg \u003d _(\u0027Node %s can not be updated while a state transition\u0027"},{"line_number":505,"context_line":"                     \u0027is in progress.\u0027)"},{"line_number":506,"context_line":"            raise wsme.exc.ClientSideError(msg % uuid, status_code\u003d409)"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9UqM%3D","line":505,"in_reply_to":"AAAATn%2F9XrA%3D","updated":"2013-12-12 17:19:33.000000000","message":"Right, I usually use double quote when the string contains a single quote, I personally think that the single quote looks clearer. I don\u0027t think the PEP8 says anything about it, is that a convention you created or based on some document?","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ea36f6b7ebe519611682bc904baa2e44be40ab4c","unresolved":false,"context_lines":[{"line_number":515,"context_line":"        # Update only the fields that have changed"},{"line_number":516,"context_line":"        for field in objects.Node.fields:"},{"line_number":517,"context_line":"            if rpc_node[field] !\u003d getattr(node, field):"},{"line_number":518,"context_line":"                if field \u003d\u003d \u0027driver\u0027:"},{"line_number":519,"context_line":"                    self._ensure_driver_is_supported(node.driver)"},{"line_number":520,"context_line":"                rpc_node[field] \u003d getattr(node, field)"},{"line_number":521,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9VUs%3D","line":518,"updated":"2013-12-12 16:20:04.000000000","message":"Would you please add comment here like: \"if delta has driver field make sure the new value is valid\"? Just currently this line seems a little extraordinary here.","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ba99d8f937e1343cb4867d7145e8680beb861c88","unresolved":false,"context_lines":[{"line_number":515,"context_line":"        # Update only the fields that have changed"},{"line_number":516,"context_line":"        for field in objects.Node.fields:"},{"line_number":517,"context_line":"            if rpc_node[field] !\u003d getattr(node, field):"},{"line_number":518,"context_line":"                if field \u003d\u003d \u0027driver\u0027:"},{"line_number":519,"context_line":"                    self._ensure_driver_is_supported(node.driver)"},{"line_number":520,"context_line":"                rpc_node[field] \u003d getattr(node, field)"},{"line_number":521,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9Ups%3D","line":518,"in_reply_to":"AAAATn%2F9VUs%3D","updated":"2013-12-12 17:19:33.000000000","message":"will do, thanks!","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"}],"ironic/tests/api/test_nodes.py":[{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"52729210ee5aefa2d2a484e54013dd492f41867f","unresolved":false,"context_lines":[{"line_number":338,"context_line":"        p \u003d mock.patch.object(rpcapi.ConductorAPI, \u0027change_node_power_state\u0027)"},{"line_number":339,"context_line":"        self.mock_cnps \u003d p.start()"},{"line_number":340,"context_line":"        self.addCleanup(p.stop)"},{"line_number":341,"context_line":"        self.dbapi.register_conductor({\u0027hostname\u0027: \u0027fake-host\u0027,"},{"line_number":342,"context_line":"                                       \u0027drivers\u0027: [\u0027fake\u0027]})"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def test_update_ok(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9Zz8%3D","line":341,"updated":"2013-12-11 21:50:15.000000000","message":"Instead of registering a conductor here, I think it is better to mock the dbapi.list_active_conductor_drivers method. Granted, there are already some functional dependencies on the dbapi layer in this test module, but I don\u0027t see a need to introduce this one.","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ba99d8f937e1343cb4867d7145e8680beb861c88","unresolved":false,"context_lines":[{"line_number":338,"context_line":"        p \u003d mock.patch.object(rpcapi.ConductorAPI, \u0027change_node_power_state\u0027)"},{"line_number":339,"context_line":"        self.mock_cnps \u003d p.start()"},{"line_number":340,"context_line":"        self.addCleanup(p.stop)"},{"line_number":341,"context_line":"        self.dbapi.register_conductor({\u0027hostname\u0027: \u0027fake-host\u0027,"},{"line_number":342,"context_line":"                                       \u0027drivers\u0027: [\u0027fake\u0027]})"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def test_update_ok(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9Uo8%3D","line":341,"in_reply_to":"AAAATn%2F9Zz8%3D","updated":"2013-12-12 17:19:33.000000000","message":"+1, will do thanks","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"4d60ba804b4c5da3a55468362198430e492f0c8f","unresolved":false,"context_lines":[{"line_number":488,"context_line":"                        [{\u0027path\u0027: \u0027/driver\u0027, \u0027value\u0027: new_driver,"},{"line_number":489,"context_line":"                          \u0027op\u0027: \u0027replace\u0027}])"},{"line_number":490,"context_line":"        self.mock_update_node.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":""},{"line_number":493,"context_line":"class TestPost(base.FunctionalTest):"},{"line_number":494,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9XUg%3D","line":491,"updated":"2013-12-12 08:14:03.000000000","message":"Lucas, I did not see the unittest code to cover the transitioning state node updating case, maybe we need it, how do you think?","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ba99d8f937e1343cb4867d7145e8680beb861c88","unresolved":false,"context_lines":[{"line_number":488,"context_line":"                        [{\u0027path\u0027: \u0027/driver\u0027, \u0027value\u0027: new_driver,"},{"line_number":489,"context_line":"                          \u0027op\u0027: \u0027replace\u0027}])"},{"line_number":490,"context_line":"        self.mock_update_node.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":""},{"line_number":493,"context_line":"class TestPost(base.FunctionalTest):"},{"line_number":494,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9UnI%3D","line":491,"in_reply_to":"AAAATn%2F9XUg%3D","updated":"2013-12-12 17:19:33.000000000","message":"Ah, so states can\u0027t be changed using patch. As changing a state (like powering on/off a machine) might take a lot of time, it\u0027s an async task and for that reason we expose states in another subresource[1].\n\n[1] https://review.openstack.org/#/c/60299/","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"52729210ee5aefa2d2a484e54013dd492f41867f","unresolved":false,"context_lines":[{"line_number":609,"context_line":"        super(TestDelete, self).setUp()"},{"line_number":610,"context_line":"        cdict \u003d dbutils.get_test_chassis()"},{"line_number":611,"context_line":"        self.chassis \u003d self.dbapi.create_chassis(cdict)"},{"line_number":612,"context_line":"        self.dbapi.register_conductor({\u0027hostname\u0027: \u0027fake-host\u0027,"},{"line_number":613,"context_line":"                                       \u0027drivers\u0027: [\u0027fake\u0027]})"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"    def test_delete_node(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9ZzM%3D","line":612,"updated":"2013-12-11 21:50:15.000000000","message":"ditto here","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ba99d8f937e1343cb4867d7145e8680beb861c88","unresolved":false,"context_lines":[{"line_number":609,"context_line":"        super(TestDelete, self).setUp()"},{"line_number":610,"context_line":"        cdict \u003d dbutils.get_test_chassis()"},{"line_number":611,"context_line":"        self.chassis \u003d self.dbapi.create_chassis(cdict)"},{"line_number":612,"context_line":"        self.dbapi.register_conductor({\u0027hostname\u0027: \u0027fake-host\u0027,"},{"line_number":613,"context_line":"                                       \u0027drivers\u0027: [\u0027fake\u0027]})"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"    def test_delete_node(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAATn%2F9Umg%3D","line":612,"in_reply_to":"AAAATn%2F9ZzM%3D","updated":"2013-12-12 17:19:33.000000000","message":"will fix :)","commit_id":"7591cb1a79a42f55d5d85e32e24510895411e8c2"}]}