)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"035e6e1d824fda31609a6c6ba1e90a276745da25","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Don\u0027t allow change reservation at create/update node"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"\u0027reservation\u0027 attribute of node must be set/clean only in"},{"line_number":10,"context_line":"specific methods, direct changing causes errors."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1250348"},{"line_number":13,"context_line":"Change-Id: I9b0d5aae7fa7742a3ae43aa725085a393f1e6eda"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"AAAATn%2F%2FIwI%3D","line":10,"updated":"2013-11-13 23:34:39.000000000","message":"Thanks for including the bug link. However per https://wiki.openstack.org/wiki/GitCommitMessages#Information_in_commit_messages we still need details in the commit message.\n\nFor instance, rather than saying \u0027, direct changing causes errors\u0027, you might stop the previous sentence at methods, and add a new sentence: \u0027The reservation attribute is used by Ironic to manage exclusive locks, if it is set via the API the node will be locked permanently with no way to unlock it.\u0027","commit_id":"3a5e5162cbe8cb28a1425994192ee0afa5674418"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"2b9ca3e7295aca4767d1d51ab0b8684ffc7f3d8d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Don\u0027t allow change reservation at create/update node"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"\u0027reservation\u0027 attribute of node must be set/clean only in"},{"line_number":10,"context_line":"specific methods, direct changing causes errors."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1250348"},{"line_number":13,"context_line":"Change-Id: I9b0d5aae7fa7742a3ae43aa725085a393f1e6eda"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"AAAATn%2F%2FHUk%3D","line":10,"in_reply_to":"AAAATn%2F%2FIwI%3D","updated":"2013-11-14 09:10:08.000000000","message":"Done","commit_id":"3a5e5162cbe8cb28a1425994192ee0afa5674418"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"a7e935e49236659eda785e9044472b9f5d0a64c3","unresolved":false,"context_lines":[{"line_number":10,"context_line":"locks. This patch allows only clean existing reservation via API."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1250348"},{"line_number":13,"context_line":"Partially implements: breaking-resource-locks"},{"line_number":14,"context_line":"Change-Id: I9b0d5aae7fa7742a3ae43aa725085a393f1e6eda"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"AAAATn%2F9hKg%3D","line":13,"updated":"2013-12-10 17:05:23.000000000","message":"\"Partially implements\" is the wrong way to tag a blueprint. Please use\n\n  blueprint breaking-resource-locks","commit_id":"6e91b92d7bca94f90caaf73e12597475afeccbd4"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"1d713a9953ad51993db195197c4c1b69e94d3986","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yuriy Zveryanskyy \u003cyzveryanskyy@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-12-26 16:04:59 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow clean reservation at update node"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The \u0027reservation\u0027 attribute is used by Ironic to manage exclusive"},{"line_number":10,"context_line":"locks. This patch allows only clean existing reservation via API"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"AAAAT3%2F%2F7Ec%3D","line":7,"updated":"2013-12-27 02:57:32.000000000","message":"I think this commit subject is a bit unclear, and would prefer something like\n\n  Add ability to break TaskManager locks via REST API","commit_id":"ec3a88ca3bf1ee5f5115d9e598852953208dd398"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"0f9ad2d746265ccd0266a602e83b5c597d700627","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yuriy Zveryanskyy \u003cyzveryanskyy@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-12-26 16:04:59 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow clean reservation at update node"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The \u0027reservation\u0027 attribute is used by Ironic to manage exclusive"},{"line_number":10,"context_line":"locks. This patch allows only clean existing reservation via API"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"AAAAT3%2F%2F1aA%3D","line":7,"in_reply_to":"AAAAT3%2F%2F7Ec%3D","updated":"2013-12-30 14:41:14.000000000","message":"Done","commit_id":"ec3a88ca3bf1ee5f5115d9e598852953208dd398"}],"ironic/api/controllers/v1/node.py":[{"author":{"_account_id":1726,"name":"Ghe Rivero","email":"ghe.rivero@gmail.com","username":"ghe.rivero"},"change_message_id":"e9bca12270a4bf03ae64924c126085a4232a76b3","unresolved":false,"context_lines":[{"line_number":481,"context_line":"            defaults \u003d objects.Node.get_defaults()"},{"line_number":482,"context_line":"            for key in defaults:"},{"line_number":483,"context_line":"                # Internal values that shouldn\u0027t be part of the patch"},{"line_number":484,"context_line":"                if key in [\u0027id\u0027, \u0027updated_at\u0027, \u0027created_at\u0027, \u0027reservation\u0027]:"},{"line_number":485,"context_line":"                    continue"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"                # In case of a remove operation, add the missing fields back"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2B8us%3D","line":484,"updated":"2013-11-18 08:28:44.000000000","message":"When trying to patch these values, we shouldn\u0027t silently drop them, Besides, according to the bug report, there should be a way to, at least,  clear the reservation value.","commit_id":"6c64ca0035dac3ce44379f368011ca7daf2325c9"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"b98c013b3846a934cd6ad7323c6760fbc8bb35cb","unresolved":false,"context_lines":[{"line_number":481,"context_line":"            defaults \u003d objects.Node.get_defaults()"},{"line_number":482,"context_line":"            for key in defaults:"},{"line_number":483,"context_line":"                # Internal values that shouldn\u0027t be part of the patch"},{"line_number":484,"context_line":"                if key in [\u0027id\u0027, \u0027updated_at\u0027, \u0027created_at\u0027, \u0027reservation\u0027]:"},{"line_number":485,"context_line":"                    continue"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"                # In case of a remove operation, add the missing fields back"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2B6rQ%3D","line":484,"in_reply_to":"AAAATn%2F%2B8us%3D","updated":"2013-11-18 15:56:44.000000000","message":"1) Maybe, but this is not purpose of this patch. 2) Done.","commit_id":"6c64ca0035dac3ce44379f368011ca7daf2325c9"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"51d9b82e4876b8d6f5cea4eb80993fa76dedb278","unresolved":false,"context_lines":[{"line_number":486,"context_line":""},{"line_number":487,"context_line":"        if patched_node.get(\u0027reservation\u0027) is not None:"},{"line_number":488,"context_line":"            raise wsme.exc.ClientSideError(_(\"Set node reservation not\""},{"line_number":489,"context_line":"                                             \" allowed.\"))"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        try:"},{"line_number":492,"context_line":"            self. _convert_chassis_uuid_to_id(patched_node)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2Bmho%3D","line":489,"updated":"2013-11-22 16:40:23.000000000","message":"1) Currently \u0027reservation\u0027 is not an attribute in the API object, so it can\u0027t be patched.\n\n2) I think that in order to break a lock the node should be in some failed/error state, otherwise it just looks too fragile.\n\nThoughts?","commit_id":"1d8b074a48515fa162bf1d65e839c08f86296f50"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f640332a955fffb7d86ed6fc0a6c0d65d83e1aa4","unresolved":false,"context_lines":[{"line_number":518,"context_line":"                    self._ensure_driver_is_supported(node.driver)"},{"line_number":519,"context_line":"                rpc_node[field] \u003d getattr(node, field)"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"        try:"},{"line_number":522,"context_line":"            new_node \u003d pecan.request.rpcapi.update_node(pecan.request.context,"},{"line_number":523,"context_line":"                                                        rpc_node)"},{"line_number":524,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F94%2Bk%3D","side":"PARENT","line":521,"updated":"2013-12-04 20:28:58.000000000","message":"I was just wondering. If the request was to clear the reservation, and the node\u0027s reservation is already cleared, there could be nothing to do here. (ie, rpc_node wasn\u0027t modified at all). Hopefully the underlying code handles it properly, and maybe this is just an optimization, but do we want to prevent it from going further at this point?","commit_id":"a905af47fa68bb315e7b680e27ed42783356b391"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e6cb8b457dcd0a2cf402f70ff18e8575aef459c6","unresolved":false,"context_lines":[{"line_number":518,"context_line":"                    self._ensure_driver_is_supported(node.driver)"},{"line_number":519,"context_line":"                rpc_node[field] \u003d getattr(node, field)"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"        try:"},{"line_number":522,"context_line":"            new_node \u003d pecan.request.rpcapi.update_node(pecan.request.context,"},{"line_number":523,"context_line":"                                                        rpc_node)"},{"line_number":524,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F9w7A%3D","side":"PARENT","line":521,"in_reply_to":"AAAATn%2F90Ck%3D","updated":"2013-12-06 08:04:26.000000000","message":"IMO this is not a bug, only optimization possible.","commit_id":"a905af47fa68bb315e7b680e27ed42783356b391"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"aadd66502e8a9a47975e72ff2cdc842790e65f04","unresolved":false,"context_lines":[{"line_number":518,"context_line":"                    self._ensure_driver_is_supported(node.driver)"},{"line_number":519,"context_line":"                rpc_node[field] \u003d getattr(node, field)"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"        try:"},{"line_number":522,"context_line":"            new_node \u003d pecan.request.rpcapi.update_node(pecan.request.context,"},{"line_number":523,"context_line":"                                                        rpc_node)"},{"line_number":524,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F90Ck%3D","side":"PARENT","line":521,"in_reply_to":"AAAATn%2F92Ts%3D","updated":"2013-12-05 16:59:56.000000000","message":"Yes, it is a general problem. Are you planning on doing this later? And/or should I open a bug about it? Or I suppose we could put a NOTE about it in the code.","commit_id":"a905af47fa68bb315e7b680e27ed42783356b391"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"27e1083854862d9640f6bb64e2f540afafcc6501","unresolved":false,"context_lines":[{"line_number":518,"context_line":"                    self._ensure_driver_is_supported(node.driver)"},{"line_number":519,"context_line":"                rpc_node[field] \u003d getattr(node, field)"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"        try:"},{"line_number":522,"context_line":"            new_node \u003d pecan.request.rpcapi.update_node(pecan.request.context,"},{"line_number":523,"context_line":"                                                        rpc_node)"},{"line_number":524,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F92Ts%3D","side":"PARENT","line":521,"in_reply_to":"AAAATn%2F94%2Bk%3D","updated":"2013-12-05 09:29:02.000000000","message":"Good catch Ruby. This is general problem, not \u0027reservation\u0027  specific. User can sent patch for a node which changes nothing (look at l525-530). I think mentioned optimization is a good idea.","commit_id":"a905af47fa68bb315e7b680e27ed42783356b391"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f007c3236caecc883e36e7d421ffe7bf1dd073c2","unresolved":false,"context_lines":[{"line_number":237,"context_line":"    \"Represent the current (not transition) provision state of the node\""},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    target_provision_state \u003d wtypes.text"},{"line_number":240,"context_line":"    \"The user modified desired provision state of the node.\""},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    driver \u003d wsme.wsattr(wtypes.text, mandatory\u003dTrue)"},{"line_number":243,"context_line":"    \"The driver responsible for controlling the node\""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F9164%3D","line":240,"updated":"2013-12-05 11:06:47.000000000","message":"reservation should be part of this attributes","commit_id":"6e91b92d7bca94f90caaf73e12597475afeccbd4"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f640332a955fffb7d86ed6fc0a6c0d65d83e1aa4","unresolved":false,"context_lines":[{"line_number":511,"context_line":"            LOG.exception(e)"},{"line_number":512,"context_line":"            raise wsme.exc.ClientSideError(_(\"Patching Error: %s\") % e)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"        # allows to clean existing node reservation"},{"line_number":515,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"},{"line_number":516,"context_line":"        if new_reservation is not None and \\"},{"line_number":517,"context_line":"                           new_reservation !\u003d rpc_node[\u0027reservation\u0027]:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F95AU%3D","line":514,"updated":"2013-12-04 20:28:58.000000000","message":"You mean \"only allow the node\u0027s reservation to be cleared (set to None)\".","commit_id":"6e91b92d7bca94f90caaf73e12597475afeccbd4"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"27e1083854862d9640f6bb64e2f540afafcc6501","unresolved":false,"context_lines":[{"line_number":511,"context_line":"            LOG.exception(e)"},{"line_number":512,"context_line":"            raise wsme.exc.ClientSideError(_(\"Patching Error: %s\") % e)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"        # allows to clean existing node reservation"},{"line_number":515,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"},{"line_number":516,"context_line":"        if new_reservation is not None and \\"},{"line_number":517,"context_line":"                           new_reservation !\u003d rpc_node[\u0027reservation\u0027]:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F92XA%3D","line":514,"in_reply_to":"AAAATn%2F95AU%3D","updated":"2013-12-05 09:29:02.000000000","message":"Good point, if will be next patches I will do.","commit_id":"6e91b92d7bca94f90caaf73e12597475afeccbd4"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f007c3236caecc883e36e7d421ffe7bf1dd073c2","unresolved":false,"context_lines":[{"line_number":512,"context_line":"            raise wsme.exc.ClientSideError(_(\"Patching Error: %s\") % e)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"        # allows to clean existing node reservation"},{"line_number":515,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"},{"line_number":516,"context_line":"        if new_reservation is not None and \\"},{"line_number":517,"context_line":"                           new_reservation !\u003d rpc_node[\u0027reservation\u0027]:"},{"line_number":518,"context_line":"            raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F9138%3D","line":515,"updated":"2013-12-05 11:06:47.000000000","message":"I think first we should add reservation as an attribute to the API node object, so that it will appear in the document when clients GET nodes/\u003cuuid\u003e. And then we can allow it to be patched.\n\nBut I don\u0027t know it looks an inconsistent behavior, maybe should we expose it as a subresource where people could trigger the removal of the lock?\n\nThoughts?","commit_id":"6e91b92d7bca94f90caaf73e12597475afeccbd4"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"5b33de0488bc8226cd87ecd5be63f330018cbfd0","unresolved":false,"context_lines":[{"line_number":512,"context_line":"            raise wsme.exc.ClientSideError(_(\"Patching Error: %s\") % e)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"        # allows to clean existing node reservation"},{"line_number":515,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"},{"line_number":516,"context_line":"        if new_reservation is not None and \\"},{"line_number":517,"context_line":"                           new_reservation !\u003d rpc_node[\u0027reservation\u0027]:"},{"line_number":518,"context_line":"            raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F91sw%3D","line":515,"in_reply_to":"AAAATn%2F9138%3D","updated":"2013-12-05 11:41:48.000000000","message":"Lucas, I think \u0027reservation\u0027 is internal conductor\u0027s attribute and should not be exposed to user. User have already *state and last_error fields for information.","commit_id":"6e91b92d7bca94f90caaf73e12597475afeccbd4"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"7a8d56160bc89ce8a724987d1161e7864495f56c","unresolved":false,"context_lines":[{"line_number":571,"context_line":"            LOG.exception(e)"},{"line_number":572,"context_line":"            raise wsme.exc.ClientSideError(_(\"Patching Error: %s\") % e)"},{"line_number":573,"context_line":""},{"line_number":574,"context_line":"        # allows to clean existing node reservation"},{"line_number":575,"context_line":"        # TODO(yuriyz): add a check to ensure it can only be cleared if"},{"line_number":576,"context_line":"        # the conductor is dead"},{"line_number":577,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F9AiM%3D","line":574,"updated":"2013-12-17 19:00:09.000000000","message":"Copying Ruby\u0027s comment from the prior patch, which was not addressed in this revision.\n\n  You mean \"only allow the node\u0027s reservation to be cleared (set to None)\".","commit_id":"ba5872a55aa82006ad2ecf7bd00c2d2013356a19"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"01a39ce64c9f912eca0bd16e4ac87e2e6c463323","unresolved":false,"context_lines":[{"line_number":571,"context_line":"            LOG.exception(e)"},{"line_number":572,"context_line":"            raise wsme.exc.ClientSideError(_(\"Patching Error: %s\") % e)"},{"line_number":573,"context_line":""},{"line_number":574,"context_line":"        # allows to clean existing node reservation"},{"line_number":575,"context_line":"        # TODO(yuriyz): add a check to ensure it can only be cleared if"},{"line_number":576,"context_line":"        # the conductor is dead"},{"line_number":577,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F88iw%3D","line":574,"in_reply_to":"AAAATn%2F9AiM%3D","updated":"2013-12-18 14:13:41.000000000","message":"Sorry, I forgot this.","commit_id":"ba5872a55aa82006ad2ecf7bd00c2d2013356a19"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c924828716d0eac7f25cda711f784d742a456347","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    # TODO(yuriyz): make \u0027reservation\u0027 read-only for users"},{"line_number":186,"context_line":"    reservation \u003d wtypes.text"},{"line_number":187,"context_line":"    \"Indicates exclusive lock of this node by one of conductors.\""},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    driver \u003d wsme.wsattr(wtypes.text, mandatory\u003dTrue)"},{"line_number":190,"context_line":"    \"The driver responsible for controlling the node\""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F87MM%3D","line":187,"updated":"2013-12-18 18:16:54.000000000","message":"Suggestion: \"Indicates\" sounds like it will hold a boolean value (1/0, True/False), maybe something like:\n\n\"The hostname of the conductor that holds an exclusive lock on the node\"\n\nwould be more precise?","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"bf30c348f0bab0551c653de22accbf88e2346a50","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    # TODO(yuriyz): make \u0027reservation\u0027 read-only for users"},{"line_number":186,"context_line":"    reservation \u003d wtypes.text"},{"line_number":187,"context_line":"    \"Indicates exclusive lock of this node by one of conductors.\""},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    driver \u003d wsme.wsattr(wtypes.text, mandatory\u003dTrue)"},{"line_number":190,"context_line":"    \"The driver responsible for controlling the node\""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F84YE%3D","line":187,"in_reply_to":"AAAATn%2F87MM%3D","updated":"2013-12-19 08:58:04.000000000","message":"Done","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c924828716d0eac7f25cda711f784d742a456347","unresolved":false,"context_lines":[{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def _get_active_conductors(self):"},{"line_number":380,"context_line":"        drivers \u003d pecan.request.dbapi.get_active_driver_dict("},{"line_number":381,"context_line":"                                    interval\u003dCONF.conductor.heartbeat_timeout)"},{"line_number":382,"context_line":"        conductors \u003d reduce(lambda x, y: x | y, drivers.values()) if drivers \\"},{"line_number":383,"context_line":"                     else []"},{"line_number":384,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F87LA%3D","line":381,"updated":"2013-12-18 18:16:54.000000000","message":"We don\u0027t need to pass this interval\u003dCONF.... anymore, by default the get_active_driver_dict() already uses this config option [1]\n\n[1] https://github.com/openstack/ironic/blob/master/ironic/db/sqlalchemy/api.py#L551-L552","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"bf30c348f0bab0551c653de22accbf88e2346a50","unresolved":false,"context_lines":[{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def _get_active_conductors(self):"},{"line_number":380,"context_line":"        drivers \u003d pecan.request.dbapi.get_active_driver_dict("},{"line_number":381,"context_line":"                                    interval\u003dCONF.conductor.heartbeat_timeout)"},{"line_number":382,"context_line":"        conductors \u003d reduce(lambda x, y: x | y, drivers.values()) if drivers \\"},{"line_number":383,"context_line":"                     else []"},{"line_number":384,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F84Xo%3D","line":381,"in_reply_to":"AAAATn%2F87LA%3D","updated":"2013-12-19 08:58:04.000000000","message":"Done","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ec61b83af36e3d410c344b123d6fa8e6a1b9b319","unresolved":false,"context_lines":[{"line_number":376,"context_line":"                    \"can only be true or false.\") % associated)"},{"line_number":377,"context_line":"        return nodes"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def _get_active_conductors(self):"},{"line_number":380,"context_line":"        drivers \u003d pecan.request.dbapi.get_active_driver_dict()"},{"line_number":381,"context_line":"        conductors \u003d six.moves.reduce(lambda x, y: x | y, drivers.values()) \\"},{"line_number":382,"context_line":"                     if drivers else []"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F8mSg%3D","line":379,"updated":"2013-12-24 15:38:30.000000000","message":"Maybe to move this method to db api level, I assume we\u0027ll widely use it, f.e. for some periodic tasks.\n\nMoreover it\u0027s a bit over-complicated now: we retrieving a heavy structure {driver: set(conductors)} to join all the conductors into a common set. Instead this, on db api level we could do:\n\n        limit \u003d timeutils.utcnow() - datetime.timedelta(seconds\u003dinterval)\n        result \u003d model_query(models.Conductor.hostname).\\\n                    filter(models.Conductor.updated_at \u003e\u003d limit).\\\n                    all()","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e01d6bdf2b5e489c9e765e7f084fc476ee1742ba","unresolved":false,"context_lines":[{"line_number":376,"context_line":"                    \"can only be true or false.\") % associated)"},{"line_number":377,"context_line":"        return nodes"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def _get_active_conductors(self):"},{"line_number":380,"context_line":"        drivers \u003d pecan.request.dbapi.get_active_driver_dict()"},{"line_number":381,"context_line":"        conductors \u003d six.moves.reduce(lambda x, y: x | y, drivers.values()) \\"},{"line_number":382,"context_line":"                     if drivers else []"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAT3%2F%2F%2FmA%3D","line":379,"in_reply_to":"AAAATn%2F8mSg%3D","updated":"2013-12-25 12:51:17.000000000","message":"Agree, I also thought about it.","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ec61b83af36e3d410c344b123d6fa8e6a1b9b319","unresolved":false,"context_lines":[{"line_number":379,"context_line":"    def _get_active_conductors(self):"},{"line_number":380,"context_line":"        drivers \u003d pecan.request.dbapi.get_active_driver_dict()"},{"line_number":381,"context_line":"        conductors \u003d six.moves.reduce(lambda x, y: x | y, drivers.values()) \\"},{"line_number":382,"context_line":"                     if drivers else []"},{"line_number":383,"context_line":""},{"line_number":384,"context_line":"        return conductors"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F8mfs%3D","line":382,"updated":"2013-12-24 15:38:30.000000000","message":"Sorry, reduce always blows my mind :) If you will prefer to leave this logic in REST API level I\u0027d very appreciate if you rewrite it like\n\n    conductors \u003d set.union(*drivers.values()) if drivers else []","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ec61b83af36e3d410c344b123d6fa8e6a1b9b319","unresolved":false,"context_lines":[{"line_number":510,"context_line":"        # if conductor which reserved this node is dead"},{"line_number":511,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"},{"line_number":512,"context_line":"        old_reservation \u003d rpc_node[\u0027reservation\u0027]"},{"line_number":513,"context_line":"        if new_reservation is not None and new_reservation !\u003d old_reservation:"},{"line_number":514,"context_line":"            raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":515,"context_line":"                                             \" allowed.\"))"},{"line_number":516,"context_line":"        if new_reservation is None and old_reservation:"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F8mas%3D","line":513,"updated":"2013-12-24 15:38:30.000000000","message":"What do you think if new_reservation it the same to old, is it normal behavior that we\u0027re allowing to re-reserve nodes via API? I mean the original reservation always happen in \"task_manager.acquire\" and is not exposed in API, but if we try to perform reservation with similar conductor via API - it will look like it\u0027s allowed to reserve via API\n\nMaybe something like:\n\n    if new_reservation is not None:\n        raise ...","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"e5537426fd406abee5e3aecea559594ab185903f","unresolved":false,"context_lines":[{"line_number":510,"context_line":"        # if conductor which reserved this node is dead"},{"line_number":511,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"},{"line_number":512,"context_line":"        old_reservation \u003d rpc_node[\u0027reservation\u0027]"},{"line_number":513,"context_line":"        if new_reservation is not None and new_reservation !\u003d old_reservation:"},{"line_number":514,"context_line":"            raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":515,"context_line":"                                             \" allowed.\"))"},{"line_number":516,"context_line":"        if new_reservation is None and old_reservation:"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAT3%2F%2F%2B94%3D","line":513,"in_reply_to":"AAAAT3%2F%2F%2FMk%3D","updated":"2013-12-25 13:34:02.000000000","message":"I overlooked apply_patch behavior. Thanks for the explanation. Glad that it makes sense :)","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e01d6bdf2b5e489c9e765e7f084fc476ee1742ba","unresolved":false,"context_lines":[{"line_number":510,"context_line":"        # if conductor which reserved this node is dead"},{"line_number":511,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"},{"line_number":512,"context_line":"        old_reservation \u003d rpc_node[\u0027reservation\u0027]"},{"line_number":513,"context_line":"        if new_reservation is not None and new_reservation !\u003d old_reservation:"},{"line_number":514,"context_line":"            raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":515,"context_line":"                                             \" allowed.\"))"},{"line_number":516,"context_line":"        if new_reservation is None and old_reservation:"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAT3%2F%2F%2FMk%3D","line":513,"in_reply_to":"AAAATn%2F8mas%3D","updated":"2013-12-25 12:51:17.000000000","message":"Max, look at l503. If patch does not touch \u0027reservation\u0027 new reservation is the same as old. But update is forbidden for locked nodes and there is race condition possible for re-reserve, therefore your comments makes sense.","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ec61b83af36e3d410c344b123d6fa8e6a1b9b319","unresolved":false,"context_lines":[{"line_number":513,"context_line":"        if new_reservation is not None and new_reservation !\u003d old_reservation:"},{"line_number":514,"context_line":"            raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":515,"context_line":"                                             \" allowed.\"))"},{"line_number":516,"context_line":"        if new_reservation is None and old_reservation:"},{"line_number":517,"context_line":"            if old_reservation not in self._get_active_conductors():"},{"line_number":518,"context_line":"                rpc_node[\u0027reservation\u0027] \u003d None"},{"line_number":519,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F8mRE%3D","line":516,"updated":"2013-12-24 15:38:30.000000000","message":"and then here we will do:\n\nif old_reservation:\n    if old_reservation not in self._get_active_conductors():\n...","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"27345725cab697cc97b05ca97e8a4d5aca4de75b","unresolved":false,"context_lines":[{"line_number":508,"context_line":"                raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":509,"context_line":"                                                 \" allowed.\"))"},{"line_number":510,"context_line":"            else:"},{"line_number":511,"context_line":"                raise exception.NodeLocked(node\u003duuid)"},{"line_number":512,"context_line":"        else:"},{"line_number":513,"context_line":"            if old_reservation:"},{"line_number":514,"context_line":"                conductors \u003d pecan.request.dbapi.get_active_conductors_list()"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAT3%2F%2F%2ByE%3D","line":511,"updated":"2013-12-25 15:45:22.000000000","message":"Sorry, I can\u0027t get it again. In previous patch you mentioned \"If patch does not touch \u0027reservation\u0027 new reservation is the same as old\"\n\nSo If the API gets a patch request with the same reservation as old it will raise NodeLocked, but I assume it should raise ClientSideError, because we don\u0027t allow to set reservation via API at all (I mean we only allow set to None).\n\nAlso, if the old reservation is None, and the patch request comes with None too, the \"new_reservation\" will be None, we will go the \"else\" and do nothing, but I would expect to see something like: \"No reservation to break\". How do you think is that reasonable?\n\nHow about:\n\n     if patch has operation to replace reservation: \n         if it\u0027s value is not None:\n             raise exception \"Set node \u0027reservation\u0027 not allowed\"\n             # Maybe better to say \"Node \u0027reservation\u0027 is allowed to be set to None only.\"\n         else:\n             if old reservation is already None:\n                 raise exception \"No reservation to break\"\n             if old reservation is owned by the alive conductor:\n                 raise exception \"Clear node \u0027reservation for active conductors not allowed.\"      \n      \n       try:\n          node \u003d Node(**jsonpatch.apply_patch(rpc_node.as_dict() ....","commit_id":"d820ff4776d8f7b34e4af5cfa00c0d59faf120fb"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"7437ac6e0b7419663fa9c659faab1849816c08a8","unresolved":false,"context_lines":[{"line_number":508,"context_line":"                raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":509,"context_line":"                                                 \" allowed.\"))"},{"line_number":510,"context_line":"            else:"},{"line_number":511,"context_line":"                raise exception.NodeLocked(node\u003duuid)"},{"line_number":512,"context_line":"        else:"},{"line_number":513,"context_line":"            if old_reservation:"},{"line_number":514,"context_line":"                conductors \u003d pecan.request.dbapi.get_active_conductors_list()"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAT3%2F%2F8yQ%3D","line":511,"in_reply_to":"AAAAT3%2F%2F%2ByE%3D","updated":"2013-12-26 14:10:21.000000000","message":"Max,\n\n\u003eif patch has operation to replace reservation\n\nIMO this work for jsonpatch\n\n\u003eSo If the API gets a patch request with the same reservation as old it will raise NodeLocked, but I assume it should raise ClientSideError\n\nUpdates for locked nodes prohibited in any cases, I dont see a problem. \n\n\u003eAlso, if the old reservation is None, and the patch request comes with None too, the \"new_reservation\" will be None, we will go the \"else\" and do nothing, but I would expect to see something like: \"No reservation to break\"\n\nDone.","commit_id":"d820ff4776d8f7b34e4af5cfa00c0d59faf120fb"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"484ab56f24427eb185dae54b49f286e9d3842f2f","unresolved":false,"context_lines":[{"line_number":508,"context_line":"                raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":509,"context_line":"                                                 \" allowed.\"))"},{"line_number":510,"context_line":"            else:"},{"line_number":511,"context_line":"                raise exception.NodeLocked(node\u003duuid)"},{"line_number":512,"context_line":"        else:"},{"line_number":513,"context_line":"            if old_reservation:"},{"line_number":514,"context_line":"                conductors \u003d pecan.request.dbapi.get_active_conductors_list()"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAT3%2F%2F8OQ%3D","line":511,"in_reply_to":"AAAAT3%2F%2F8yQ%3D","updated":"2013-12-26 17:21:55.000000000","message":"\u003eIMO this work for jsonpatch\n\nDid you mean \"won\u0027t work\"? I think it will be much easier to handle all these problems if we parse jsonpatch and check \"if patch has operation to replace reservation\", e.g. write like in my previous comment. We could have a separate utils class, or extend jsonpatch class, or even contribute to jsonpatch to have this.\n\n\u003eUpdates for locked nodes prohibited in any cases, I dont see a problem.\n\nI just don\u0027t like NodeLocked for this case. It\u0027s like: \"reservation is allowed, but the node is already locked, so your request failed\". But it should be \"reservation is not allowed\"","commit_id":"d820ff4776d8f7b34e4af5cfa00c0d59faf120fb"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"1d713a9953ad51993db195197c4c1b69e94d3986","unresolved":false,"context_lines":[{"line_number":499,"context_line":"            LOG.exception(e)"},{"line_number":500,"context_line":"            raise wsme.exc.ClientSideError(_(\"Patching Error: %s\") % e)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"        # only allow the node\u0027s reservation to be cleared (set to None)"},{"line_number":503,"context_line":"        # if conductor which reserved this node is dead"},{"line_number":504,"context_line":"        old_reservation \u003d rpc_node[\u0027reservation\u0027]"},{"line_number":505,"context_line":"        if old_reservation is None and \u0027reservation\u0027 not in patched_node:"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAT3%2F%2F7Fc%3D","line":502,"updated":"2013-12-27 02:57:32.000000000","message":"The logic here is pretty hard to follow. I think the discussion on the previous patchset was very good and relevant.","commit_id":"ec3a88ca3bf1ee5f5115d9e598852953208dd398"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"2fce2fb2f852a26691ed9f10bece941d52c8efb0","unresolved":false,"context_lines":[{"line_number":505,"context_line":"        if old_reservation is None and \u0027reservation\u0027 not in patched_node:"},{"line_number":506,"context_line":"            raise wsme.exc.ClientSideError(_(\"No reservation to break.\"))"},{"line_number":507,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"},{"line_number":508,"context_line":"        if new_reservation is not None:"},{"line_number":509,"context_line":"            if new_reservation !\u003d old_reservation:"},{"line_number":510,"context_line":"                raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":511,"context_line":"                                                 \" allowed.\"))"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAT3%2F%2F7V4%3D","line":508,"updated":"2013-12-27 00:01:00.000000000","message":"Here, we can say more straightforward as below:\n\nif new_reservation:\n    #do","commit_id":"ec3a88ca3bf1ee5f5115d9e598852953208dd398"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"0f9ad2d746265ccd0266a602e83b5c597d700627","unresolved":false,"context_lines":[{"line_number":505,"context_line":"        if old_reservation is None and \u0027reservation\u0027 not in patched_node:"},{"line_number":506,"context_line":"            raise wsme.exc.ClientSideError(_(\"No reservation to break.\"))"},{"line_number":507,"context_line":"        new_reservation \u003d patched_node.get(\u0027reservation\u0027)"},{"line_number":508,"context_line":"        if new_reservation is not None:"},{"line_number":509,"context_line":"            if new_reservation !\u003d old_reservation:"},{"line_number":510,"context_line":"                raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":511,"context_line":"                                                 \" allowed.\"))"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAT3%2F%2F1aI%3D","line":508,"in_reply_to":"AAAAT3%2F%2F7V4%3D","updated":"2013-12-30 14:41:14.000000000","message":"Done","commit_id":"ec3a88ca3bf1ee5f5115d9e598852953208dd398"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"cd1d70b0a5bd4cc62f11c4e6375295cb31c4b064","unresolved":false,"context_lines":[{"line_number":63,"context_line":"                                             \" allowed.\"))"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        return types.JsonPatchType.validate(patch)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"class NodeStates(base.APIBase):"},{"line_number":69,"context_line":"    \"\"\"API representation of the states of a node.\"\"\""}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAUH%2F%2BjgM%3D","line":66,"updated":"2014-01-24 14:17:59.000000000","message":"I know this reduces complexity, but I\u0027d move this code back to the controller, just to have all our \u0027rules\u0027 in one place. Please see my comment to controller.","commit_id":"af001092e9f7dce8e72b4a8824aeb7f25c40ed4e"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"d33c4013efa16074f858d360a016dcaf1534a250","unresolved":false,"context_lines":[{"line_number":586,"context_line":"                                        \" for active conductors not allowed.\"))"},{"line_number":587,"context_line":""},{"line_number":588,"context_line":"        node \u003d Node(**patched_node)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        # Update only the fields that have changed"},{"line_number":591,"context_line":"        for field in objects.Node.fields:"},{"line_number":592,"context_line":"            if rpc_node[field] !\u003d getattr(node, field):"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAUH%2F%2BFn4%3D","line":589,"updated":"2014-01-30 09:10:49.000000000","message":"After discussion I decided to return to code like in old patchsets.","commit_id":"af001092e9f7dce8e72b4a8824aeb7f25c40ed4e"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"a230b797b8e230089900d350f4c2c08b3de376f3","unresolved":false,"context_lines":[{"line_number":586,"context_line":"                                        \" for active conductors not allowed.\"))"},{"line_number":587,"context_line":""},{"line_number":588,"context_line":"        node \u003d Node(**patched_node)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        # Update only the fields that have changed"},{"line_number":591,"context_line":"        for field in objects.Node.fields:"},{"line_number":592,"context_line":"            if rpc_node[field] !\u003d getattr(node, field):"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAUH%2F%2BeYk%3D","line":589,"updated":"2014-01-25 12:59:48.000000000","message":"Max, Devananda\n\n1) Multiple operations for the same path possible in one jsonpatch (example here https://python-json-patch.readthedocs.org/en/latest/mod-jsonpatch.html). Necessity of check all operations will increase Ironic code complexity. Therefore this patch works with patched node.\n\n2) Necessity to have separate value(s) like \u0027none\u0027 for cleaning field via replace increase complexity also. This patch does\u0027nt allow \u0027replace\u0027 reservation at all.\n\nand thanks for comments, if you will not agree with me I will change this.","commit_id":"af001092e9f7dce8e72b4a8824aeb7f25c40ed4e"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"cd1d70b0a5bd4cc62f11c4e6375295cb31c4b064","unresolved":false,"context_lines":[{"line_number":586,"context_line":"                                        \" for active conductors not allowed.\"))"},{"line_number":587,"context_line":""},{"line_number":588,"context_line":"        node \u003d Node(**patched_node)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        # Update only the fields that have changed"},{"line_number":591,"context_line":"        for field in objects.Node.fields:"},{"line_number":592,"context_line":"            if rpc_node[field] !\u003d getattr(node, field):"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAUH%2F%2BjKU%3D","line":589,"updated":"2014-01-24 14:17:59.000000000","message":"So you insist on working with patched node instead of patch:) Yes, this will work for current case, but imagine if we will have the following:\n\n1. The rpc node has reservation set to None.\n\n2. To break the lock we have agreed to use the following patch:\n{\u0027op\u0027:\u0027replace\u0027, \u0027value\u0027: \u0027none\u0027, \u0027path\u0027: \u0027reservation\u0027}\n\nHow would we check whether the reservation in patched_node was set to None by the patch? (It was None from the beginnig)\n \nPlease take a look on the jsonpatch utility example (how I imagine it) and how it can help us to make it easier:\nhttp://paste.openstack.org/show/61821/","commit_id":"af001092e9f7dce8e72b4a8824aeb7f25c40ed4e"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"fc83813f35e4142ede34d734937c1d4a69de92ef","unresolved":false,"context_lines":[{"line_number":586,"context_line":"                                        \" for active conductors not allowed.\"))"},{"line_number":587,"context_line":""},{"line_number":588,"context_line":"        node \u003d Node(**patched_node)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        # Update only the fields that have changed"},{"line_number":591,"context_line":"        for field in objects.Node.fields:"},{"line_number":592,"context_line":"            if rpc_node[field] !\u003d getattr(node, field):"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAUH%2F%2BeVQ%3D","line":589,"in_reply_to":"AAAAUH%2F%2BeYk%3D","updated":"2014-01-25 13:40:01.000000000","message":"Good points!\n\nfor 2) - I posted replace approach just as example to show that current solution decreases our flexibility.\n\nfor 1) - very true. But I guess if at least one operation will represent the restricted action we already can throw the error to the client, even if the total result of the patch will eliminate this action.\n\nSo the behavior will be:\n\n1. let\u0027s assume the patch has 5 operations, 2nd and 4th are not supported by our service\n\n2. we\u0027re iterating till 2nd and throw exception to the client saying the 2nd is restricted (even if 3rd will eliminate 2nd, still the second is restricted).\n\n3. the user fixes 2nd operation and sending again, with only 4th broken.\n\n4. we\u0027re iterating till 4th and throw exception again, saying the 4th is restricted.\n\n5. the user fixes 4th and sending again\n\n6. we\u0027re iterating through them all, everything turns to be allowed, we\u0027re performing required patch and sending 200\n\nI think it\u0027s common behavior for most of the services, right? Any thoughts?","commit_id":"af001092e9f7dce8e72b4a8824aeb7f25c40ed4e"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"e6f6bcdedfab296c47136af6f24480fb516ae39b","unresolved":false,"context_lines":[{"line_number":586,"context_line":"                                        \" for active conductors not allowed.\"))"},{"line_number":587,"context_line":""},{"line_number":588,"context_line":"        node \u003d Node(**patched_node)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        # Update only the fields that have changed"},{"line_number":591,"context_line":"        for field in objects.Node.fields:"},{"line_number":592,"context_line":"            if rpc_node[field] !\u003d getattr(node, field):"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAUH%2F%2Bi%2Bw%3D","line":589,"in_reply_to":"AAAAUH%2F%2BjKU%3D","updated":"2014-01-24 15:21:02.000000000","message":"Max, thanks for the examples -- that\u0027s what I had in mind with my previous comments.","commit_id":"af001092e9f7dce8e72b4a8824aeb7f25c40ed4e"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d641276a1792e9e876a58a40aa760d1a85f6c143","unresolved":false,"context_lines":[{"line_number":451,"context_line":"        return nodes"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    @staticmethod"},{"line_number":454,"context_line":"    def _check_clear_reservation(rpc_node, patched_node):"},{"line_number":455,"context_line":"        if rpc_node.reservation:"},{"line_number":456,"context_line":"            if \u0027reservation\u0027 not in patched_node:"},{"line_number":457,"context_line":"                conductors \u003d pecan.request.dbapi.get_active_conductors_list()"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAUH%2F99bI%3D","line":454,"updated":"2014-01-31 17:11:16.000000000","message":"Hi yuriy,\n\nI don\u0027t see any reason why this method has to be static, private (\u0027_\u0027 prefixed) and static looks a bit odd, because it\u0027s going to be accessed like NodesController._check_clear_reservation(...), it\u0027s like accessing a private method from outside.\n\nI would remove the static from this method and just threat it as a normal method of the class.","commit_id":"47156deb69e6df93e91f9ab0bbf2ae9794096ab4"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"667865f87d2ab7f99c1b92c01180202638bac988","unresolved":false,"context_lines":[{"line_number":451,"context_line":"        return nodes"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    @staticmethod"},{"line_number":454,"context_line":"    def _check_clear_reservation(rpc_node, patched_node):"},{"line_number":455,"context_line":"        if rpc_node.reservation:"},{"line_number":456,"context_line":"            if \u0027reservation\u0027 not in patched_node:"},{"line_number":457,"context_line":"                conductors \u003d pecan.request.dbapi.get_active_conductors_list()"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAUH%2F%2BBeE%3D","line":454,"updated":"2014-01-30 22:18:02.000000000","message":"In addition to comment below, please add a doc string to this method. Something like this would be helpful.\n\n\n  \"\"\"Enforce rules on when the \u0027reservation\u0027 property may be changed.\n\n  Only allow \u0027reservation\u0027 to be cleared (unset) if the referenced conductor\n  is no longer active. Do not otherwise allow \u0027reservation\u0027 to ever be set to\n  a non-None value, or changed.\n\n  :param rpc_node: an unchanged Node object\n  :param patched_node: a JSONPatch object representing the new object\n  :raises: wsme.exc.ClientSideError\n\n  \"\"\"\n\nThanks!","commit_id":"47156deb69e6df93e91f9ab0bbf2ae9794096ab4"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"02ffb8c0b87266366a7ee6c72c38c1601dec4b82","unresolved":false,"context_lines":[{"line_number":451,"context_line":"        return nodes"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    @staticmethod"},{"line_number":454,"context_line":"    def _check_clear_reservation(rpc_node, patched_node):"},{"line_number":455,"context_line":"        if rpc_node.reservation:"},{"line_number":456,"context_line":"            if \u0027reservation\u0027 not in patched_node:"},{"line_number":457,"context_line":"                conductors \u003d pecan.request.dbapi.get_active_conductors_list()"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAUH%2F%2BApc%3D","line":454,"in_reply_to":"AAAAUH%2F%2BBeE%3D","updated":"2014-01-31 00:45:08.000000000","message":"Done","commit_id":"47156deb69e6df93e91f9ab0bbf2ae9794096ab4"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d641276a1792e9e876a58a40aa760d1a85f6c143","unresolved":false,"context_lines":[{"line_number":459,"context_line":"                    raise wsme.exc.ClientSideError(_(\"Clear node \u0027reservation\u0027\""},{"line_number":460,"context_line":"                                        \" for active conductors not allowed.\"))"},{"line_number":461,"context_line":"            else:"},{"line_number":462,"context_line":"                patched_node[\u0027reservation\u0027] \u003d rpc_node.reservation"},{"line_number":463,"context_line":"        elif patched_node.get(\u0027reservation\u0027):"},{"line_number":464,"context_line":"            raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":465,"context_line":"                                             \" allowed.\"))"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAUH%2F99Xo%3D","line":462,"updated":"2014-01-31 17:11:16.000000000","message":"I don\u0027t understand this else here. And if you remove this block of code all the tests will pass, so it\u0027s not being covered by the tests.\n\nBut the logic seems weird, if the reservation field is populated in the db (if rpc_node.reservation), but the patched_node doesn\u0027t contain the reservation field, so it\u0027s adding the value that is in the db to the patched node? Is it because the user could have send a patch to remove this value!?\n\nI mean this method is suppose to check things, not add values to the objects, the logic is very confusing here.","commit_id":"47156deb69e6df93e91f9ab0bbf2ae9794096ab4"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d641276a1792e9e876a58a40aa760d1a85f6c143","unresolved":false,"context_lines":[{"line_number":464,"context_line":"            raise wsme.exc.ClientSideError(_(\"Set node \u0027reservation\u0027 not\""},{"line_number":465,"context_line":"                                             \" allowed.\"))"},{"line_number":466,"context_line":"        elif \u0027reservation\u0027 not in patched_node:"},{"line_number":467,"context_line":"            raise wsme.exc.ClientSideError(_(\"No reservation to break.\"))"},{"line_number":468,"context_line":""},{"line_number":469,"context_line":"    @wsme_pecan.wsexpose(NodeCollection, types.uuid, types.uuid,"},{"line_number":470,"context_line":"               wtypes.text, types.uuid, int, wtypes.text, wtypes.text)"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAUH%2F99WA%3D","line":467,"updated":"2014-01-31 17:11:16.000000000","message":"I think we should add some tests to exercise this method specifically, there\u0027s a lot of logic here and we want to guarantee that changes to this method in the future won\u0027t break the logic","commit_id":"47156deb69e6df93e91f9ab0bbf2ae9794096ab4"}],"ironic/conductor/manager.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"51d9b82e4876b8d6f5cea4eb80993fa76dedb278","unresolved":false,"context_lines":[{"line_number":130,"context_line":"                \"Invalid method call: update_node can not change node state.\"))"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        unlock \u003d (\u0027reservation\u0027 in delta"},{"line_number":133,"context_line":"                                and node_obj.get(\u0027reservation\u0027) is None)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        driver_name \u003d node_obj.get(\u0027driver\u0027) if \u0027driver\u0027 in delta else None"},{"line_number":136,"context_line":"        with task_manager.acquire(context, node_id, shared\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2Bmio%3D","line":133,"updated":"2013-11-22 16:40:23.000000000","message":"ditto about breaking the lock only when the node is on some failed/error state","commit_id":"1d8b074a48515fa162bf1d65e839c08f86296f50"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"609569f44e67f5cac35b46af68c497280f63e80c","unresolved":false,"context_lines":[{"line_number":130,"context_line":"                \"Invalid method call: update_node can not change node state.\"))"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        unlock \u003d (\u0027reservation\u0027 in delta"},{"line_number":133,"context_line":"                                and node_obj.get(\u0027reservation\u0027) is None)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        driver_name \u003d node_obj.get(\u0027driver\u0027) if \u0027driver\u0027 in delta else None"},{"line_number":136,"context_line":"        with task_manager.acquire(context, node_id, shared\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2BhAg%3D","line":133,"in_reply_to":"AAAATn%2F%2Bmio%3D","updated":"2013-11-25 09:12:52.000000000","message":"Lucas, let\u0027s consider the following situation:\n\n1) Conductor starts new power/deploy operation.\n\n2) Conductor crashed.\n\nA node will be w/o failed/error state and locked.","commit_id":"1d8b074a48515fa162bf1d65e839c08f86296f50"}],"ironic/db/api.py":[{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ec61b83af36e3d410c344b123d6fa8e6a1b9b319","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        \"\"\"Release the reservation on a set of nodes atomically."},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        :param tag: A string uniquely identifying the reservation holder."},{"line_number":104,"context_line":"                    Release any reservation if tag is false."},{"line_number":105,"context_line":"        :param nodes: A list of node id or uuid."},{"line_number":106,"context_line":"        :raises: NodeNotFound if any node is not found."},{"line_number":107,"context_line":"        :raises: NodeAlreadyReserved if any node could not be released"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F8mQE%3D","line":104,"updated":"2013-12-24 15:38:30.000000000","message":"It seems this should be:\n\nRelease any reservation if tag is None.","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e01d6bdf2b5e489c9e765e7f084fc476ee1742ba","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        \"\"\"Release the reservation on a set of nodes atomically."},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        :param tag: A string uniquely identifying the reservation holder."},{"line_number":104,"context_line":"                    Release any reservation if tag is false."},{"line_number":105,"context_line":"        :param nodes: A list of node id or uuid."},{"line_number":106,"context_line":"        :raises: NodeNotFound if any node is not found."},{"line_number":107,"context_line":"        :raises: NodeAlreadyReserved if any node could not be released"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAT3%2F%2F%2FEY%3D","line":104,"in_reply_to":"AAAATn%2F8mQE%3D","updated":"2013-12-25 12:51:17.000000000","message":"Done","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"27345725cab697cc97b05ca97e8a4d5aca4de75b","unresolved":false,"context_lines":[{"line_number":352,"context_line":"        \"\"\"Retrieve list of hostnames for the registered and active conductors."},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"        :param interval: Seconds since last check-in of a conductor."},{"line_number":355,"context_line":"        :returns: List of hostnames for the registered and active conductors."},{"line_number":356,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAT3%2F%2F%2BwU%3D","line":355,"updated":"2013-12-25 15:45:22.000000000","message":"List of hostnames of the conductors which are registered and active during the last \u003cinterval\u003e seconds\n\nThis seems more clear to me. If you like it I thing we also need to do something with the comment for get_active_driver_dict, e.g.\n\nA dict which maps driver names to the set of hosts which support them and were active active during the last \u003cinterval\u003e seconds.  For example:\t{driverA: set...","commit_id":"d820ff4776d8f7b34e4af5cfa00c0d59faf120fb"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"7437ac6e0b7419663fa9c659faab1849816c08a8","unresolved":false,"context_lines":[{"line_number":352,"context_line":"        \"\"\"Retrieve list of hostnames for the registered and active conductors."},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"        :param interval: Seconds since last check-in of a conductor."},{"line_number":355,"context_line":"        :returns: List of hostnames for the registered and active conductors."},{"line_number":356,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAT3%2F%2F80g%3D","line":355,"in_reply_to":"AAAAT3%2F%2F%2BwU%3D","updated":"2013-12-26 14:10:21.000000000","message":"Done","commit_id":"d820ff4776d8f7b34e4af5cfa00c0d59faf120fb"}],"ironic/db/sqlalchemy/api.py":[{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"27345725cab697cc97b05ca97e8a4d5aca4de75b","unresolved":false,"context_lines":[{"line_number":175,"context_line":"    raise exception.NodeNotFound(node\u003dmissing.pop())"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"def _get_active_conductors(self, interval\u003dNone):"},{"line_number":179,"context_line":"    if interval is None:"},{"line_number":180,"context_line":"        interval \u003d CONF.conductor.heartbeat_timeout"},{"line_number":181,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAT3%2F%2F%2Bvg%3D","line":178,"updated":"2013-12-25 15:45:22.000000000","message":"I\u0027m just wondering, why we place all these utility methods directly to the module, but not to the Connection class. Perhaps you know :)","commit_id":"d820ff4776d8f7b34e4af5cfa00c0d59faf120fb"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"7437ac6e0b7419663fa9c659faab1849816c08a8","unresolved":false,"context_lines":[{"line_number":175,"context_line":"    raise exception.NodeNotFound(node\u003dmissing.pop())"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"def _get_active_conductors(self, interval\u003dNone):"},{"line_number":179,"context_line":"    if interval is None:"},{"line_number":180,"context_line":"        interval \u003d CONF.conductor.heartbeat_timeout"},{"line_number":181,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAT3%2F%2F80U%3D","line":178,"in_reply_to":"AAAAT3%2F%2F%2Bvg%3D","updated":"2013-12-26 14:10:21.000000000","message":"I dont know :) Only did as is.","commit_id":"d820ff4776d8f7b34e4af5cfa00c0d59faf120fb"}],"ironic/tests/api/test_nodes.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f640332a955fffb7d86ed6fc0a6c0d65d83e1aa4","unresolved":false,"context_lines":[{"line_number":492,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":493,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"},{"line_number":494,"context_line":"                                   \u0027op\u0027: \u0027replace\u0027}], expect_errors\u003dTrue)"},{"line_number":495,"context_line":"        self.assertEqual(response.status_code, 400)"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"class TestPost(base.FunctionalTest):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F95A8%3D","line":495,"updated":"2013-12-04 20:28:58.000000000","message":"Is it worth adding a test that shows that clearing a reservation makes it through the API part of the code? (maybe too much work to do)","commit_id":"6e91b92d7bca94f90caaf73e12597475afeccbd4"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"aadd66502e8a9a47975e72ff2cdc842790e65f04","unresolved":false,"context_lines":[{"line_number":492,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":493,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"},{"line_number":494,"context_line":"                                   \u0027op\u0027: \u0027replace\u0027}], expect_errors\u003dTrue)"},{"line_number":495,"context_line":"        self.assertEqual(response.status_code, 400)"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"class TestPost(base.FunctionalTest):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F90Bg%3D","line":495,"in_reply_to":"AAAATn%2F92Xw%3D","updated":"2013-12-05 16:59:56.000000000","message":"It isn\u0027t a big deal and I\u0027m sure it works, but it was more for thoroughness of the tests. You tested cases where the api layer stops the request, but there isn\u0027t anything to show that the code recognizes a legitimate reservation change and makes the rpcapi call (pecan.request.rpcapi.update_node). I think most of the other tests here don\u0027t test this case either.\n\nThe test_manager doesn\u0027t actually test that the code in ../controllers/v1/node.py calls pecan.request.rpcapi.update_node().\n\nBasically, if the logic surrounding the reservation code in node.py is incorrect, it may not trigger the rpcapi call. (But we\u0027ve code reviewed it and it is correct. We just don\u0027t know if someone will change the code in the future.)\n\nAnyway, that\u0027s why I asked :-)","commit_id":"6e91b92d7bca94f90caaf73e12597475afeccbd4"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"27e1083854862d9640f6bb64e2f540afafcc6501","unresolved":false,"context_lines":[{"line_number":492,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":493,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"},{"line_number":494,"context_line":"                                   \u0027op\u0027: \u0027replace\u0027}], expect_errors\u003dTrue)"},{"line_number":495,"context_line":"        self.assertEqual(response.status_code, 400)"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"class TestPost(base.FunctionalTest):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F92Xw%3D","line":495,"in_reply_to":"AAAATn%2F95A8%3D","updated":"2013-12-05 09:29:02.000000000","message":"Ruby, mocked rpcapi does not make sense here IMO. And we have this test already in test_manager.","commit_id":"6e91b92d7bca94f90caaf73e12597475afeccbd4"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"7a8d56160bc89ce8a724987d1161e7864495f56c","unresolved":false,"context_lines":[{"line_number":483,"context_line":"        self.assertEqual(response.status_code, 400)"},{"line_number":484,"context_line":"        self.assertTrue(response.json[\u0027error_message\u0027])"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    def test_set_node_reservation(self):"},{"line_number":487,"context_line":"        self.assertRaises(webtest.app.AppError, self.patch_json,"},{"line_number":488,"context_line":"                          \u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":489,"context_line":"                          [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F9Aus%3D","line":486,"updated":"2013-12-17 19:00:09.000000000","message":"Should also include a unit test that clears an existing reservation and doesn\u0027t generate an error.","commit_id":"ba5872a55aa82006ad2ecf7bd00c2d2013356a19"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"01a39ce64c9f912eca0bd16e4ac87e2e6c463323","unresolved":false,"context_lines":[{"line_number":483,"context_line":"        self.assertEqual(response.status_code, 400)"},{"line_number":484,"context_line":"        self.assertTrue(response.json[\u0027error_message\u0027])"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    def test_set_node_reservation(self):"},{"line_number":487,"context_line":"        self.assertRaises(webtest.app.AppError, self.patch_json,"},{"line_number":488,"context_line":"                          \u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":489,"context_line":"                          [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F88io%3D","line":486,"in_reply_to":"AAAATn%2F9Aus%3D","updated":"2013-12-18 14:13:41.000000000","message":"Done","commit_id":"ba5872a55aa82006ad2ecf7bd00c2d2013356a19"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c924828716d0eac7f25cda711f784d742a456347","unresolved":false,"context_lines":[{"line_number":471,"context_line":"        self.assertRaises(webtest.app.AppError, self.patch_json,"},{"line_number":472,"context_line":"                          \u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":473,"context_line":"                          [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"},{"line_number":474,"context_line":"                          \u0027op\u0027: \u0027replace\u0027}])"},{"line_number":475,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":476,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"},{"line_number":477,"context_line":"                                   \u0027op\u0027: \u0027replace\u0027}], expect_errors\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F87Qk%3D","line":474,"updated":"2013-12-18 18:16:54.000000000","message":"Wondering if necessary to have this assertRaises, we already have the patch_json with the expect_errors\u003dTrue flag on and we are also testing the return code.\n\nIf you think so, we might want to put the json-patch document in a variable and just pass it on both calls instead of duplicating it\n\ne.g\n\npatch \u003d [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027, \u0027op\u0027: \u0027replace\u0027}]\n\nself.assertRaises(webtest.app.AppError, self.patch_json, patch)\n\nresponse \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027], patch, expect_errors\u003dTrue)","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"bf30c348f0bab0551c653de22accbf88e2346a50","unresolved":false,"context_lines":[{"line_number":471,"context_line":"        self.assertRaises(webtest.app.AppError, self.patch_json,"},{"line_number":472,"context_line":"                          \u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":473,"context_line":"                          [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"},{"line_number":474,"context_line":"                          \u0027op\u0027: \u0027replace\u0027}])"},{"line_number":475,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":476,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"},{"line_number":477,"context_line":"                                   \u0027op\u0027: \u0027replace\u0027}], expect_errors\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F84TU%3D","line":474,"in_reply_to":"AAAATn%2F87Qk%3D","updated":"2013-12-19 08:58:04.000000000","message":"Done","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c924828716d0eac7f25cda711f784d742a456347","unresolved":false,"context_lines":[{"line_number":479,"context_line":""},{"line_number":480,"context_line":"    def test_clear_node_reservation_conductor_active(self):"},{"line_number":481,"context_line":"        self.dbapi.register_conductor({\u0027hostname\u0027: \u0027fake-host\u0027,"},{"line_number":482,"context_line":"                                       \u0027drivers\u0027: [\u0027drv1\u0027, \u0027drv2\u0027]})"},{"line_number":483,"context_line":""},{"line_number":484,"context_line":"        ndict \u003d dbutils.get_test_node(reservation\u003d\u0027fake-host\u0027, id\u003d345,"},{"line_number":485,"context_line":"                                      uuid\u003dutils.generate_uuid())"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F87RM%3D","line":482,"updated":"2013-12-18 18:16:54.000000000","message":"Let\u0027s just mock the \"get_active_driver_dict\" instead as per suggestion at [1]\n\ne.g\n\ndef test_clear_node_reservation_conductor_active(self):\n    with mock.patch.object(self.dbapi, \u0027get_active_driver_dict\u0027) as l_mock:\n        l_mock.return_value \u003d {\u0027drv1\u0027: set([\u0027fake-host\u0027])}\n        ndict \u003d dbutils.get_test_node(reservation\u003d\u0027fake-host\u0027, id\u003d345,\n                                                    uuid\u003dutils.generate_uuid())\n        node \u003d self.dbapi.create_node(ndict)\n        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % node[\u0027uuid\u0027],\n                                                [{\u0027path\u0027: \u0027/reservation\u0027, \u0027op\u0027: \u0027remove\u0027}],\n                                                expect_errors\u003dTrue)\n        self.assertEqual(response.status_code, 400)\n\n[1] https://review.openstack.org/#/c/57495/21/ironic/tests/api/test_nodes.py (l341)\n\nThanks","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"bf30c348f0bab0551c653de22accbf88e2346a50","unresolved":false,"context_lines":[{"line_number":479,"context_line":""},{"line_number":480,"context_line":"    def test_clear_node_reservation_conductor_active(self):"},{"line_number":481,"context_line":"        self.dbapi.register_conductor({\u0027hostname\u0027: \u0027fake-host\u0027,"},{"line_number":482,"context_line":"                                       \u0027drivers\u0027: [\u0027drv1\u0027, \u0027drv2\u0027]})"},{"line_number":483,"context_line":""},{"line_number":484,"context_line":"        ndict \u003d dbutils.get_test_node(reservation\u003d\u0027fake-host\u0027, id\u003d345,"},{"line_number":485,"context_line":"                                      uuid\u003dutils.generate_uuid())"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F84Qs%3D","line":482,"in_reply_to":"AAAATn%2F87RM%3D","updated":"2013-12-19 08:58:04.000000000","message":"Done","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c924828716d0eac7f25cda711f784d742a456347","unresolved":false,"context_lines":[{"line_number":487,"context_line":""},{"line_number":488,"context_line":"        self.assertRaises(webtest.app.AppError, self.patch_json,"},{"line_number":489,"context_line":"                          \u0027/nodes/%s\u0027 % node[\u0027uuid\u0027],"},{"line_number":490,"context_line":"                          [{\u0027path\u0027: \u0027/reservation\u0027, \u0027op\u0027: \u0027remove\u0027}])"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % node[\u0027uuid\u0027],"},{"line_number":493,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027op\u0027: \u0027remove\u0027}],"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F87QE%3D","line":490,"updated":"2013-12-18 18:16:54.000000000","message":"same question re expect_errors\u003dTrue already does it","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"bf30c348f0bab0551c653de22accbf88e2346a50","unresolved":false,"context_lines":[{"line_number":487,"context_line":""},{"line_number":488,"context_line":"        self.assertRaises(webtest.app.AppError, self.patch_json,"},{"line_number":489,"context_line":"                          \u0027/nodes/%s\u0027 % node[\u0027uuid\u0027],"},{"line_number":490,"context_line":"                          [{\u0027path\u0027: \u0027/reservation\u0027, \u0027op\u0027: \u0027remove\u0027}])"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % node[\u0027uuid\u0027],"},{"line_number":493,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027op\u0027: \u0027remove\u0027}],"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F84TA%3D","line":490,"in_reply_to":"AAAATn%2F87QE%3D","updated":"2013-12-19 08:58:04.000000000","message":"Done","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c924828716d0eac7f25cda711f784d742a456347","unresolved":false,"context_lines":[{"line_number":496,"context_line":""},{"line_number":497,"context_line":"    def test_clear_node_reservation_conductor_ok(self):"},{"line_number":498,"context_line":"        self.dbapi.register_conductor({\u0027hostname\u0027: \u0027fake-host\u0027,"},{"line_number":499,"context_line":"                                       \u0027drivers\u0027: [\u0027drv1\u0027, \u0027drv2\u0027]})"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        ndict \u003d dbutils.get_test_node(reservation\u003d\u0027fake-host-2\u0027, id\u003d345,"},{"line_number":502,"context_line":"                                      uuid\u003dutils.generate_uuid())"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F87QI%3D","line":499,"updated":"2013-12-18 18:16:54.000000000","message":"ditto re mock the \"get_active_driver_dict\"","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"bf30c348f0bab0551c653de22accbf88e2346a50","unresolved":false,"context_lines":[{"line_number":496,"context_line":""},{"line_number":497,"context_line":"    def test_clear_node_reservation_conductor_ok(self):"},{"line_number":498,"context_line":"        self.dbapi.register_conductor({\u0027hostname\u0027: \u0027fake-host\u0027,"},{"line_number":499,"context_line":"                                       \u0027drivers\u0027: [\u0027drv1\u0027, \u0027drv2\u0027]})"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        ndict \u003d dbutils.get_test_node(reservation\u003d\u0027fake-host-2\u0027, id\u003d345,"},{"line_number":502,"context_line":"                                      uuid\u003dutils.generate_uuid())"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F84Qo%3D","line":499,"in_reply_to":"AAAATn%2F87QI%3D","updated":"2013-12-19 08:58:04.000000000","message":"Done","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c924828716d0eac7f25cda711f784d742a456347","unresolved":false,"context_lines":[{"line_number":505,"context_line":""},{"line_number":506,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % node[\u0027uuid\u0027],"},{"line_number":507,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027op\u0027: \u0027remove\u0027}],"},{"line_number":508,"context_line":"                                    expect_errors\u003dTrue)"},{"line_number":509,"context_line":"        self.assertEqual(response.content_type, \u0027application/json\u0027)"},{"line_number":510,"context_line":"        self.assertEqual(response.status_code, 200)"},{"line_number":511,"context_line":"        self.mock_update_node.assert_called_once_with(mock.ANY, mock.ANY)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F87PU%3D","line":508,"updated":"2013-12-18 18:16:54.000000000","message":"expect_errors\u003dFalse or just remove it (default is False)","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"bf30c348f0bab0551c653de22accbf88e2346a50","unresolved":false,"context_lines":[{"line_number":505,"context_line":""},{"line_number":506,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % node[\u0027uuid\u0027],"},{"line_number":507,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027op\u0027: \u0027remove\u0027}],"},{"line_number":508,"context_line":"                                    expect_errors\u003dTrue)"},{"line_number":509,"context_line":"        self.assertEqual(response.content_type, \u0027application/json\u0027)"},{"line_number":510,"context_line":"        self.assertEqual(response.status_code, 200)"},{"line_number":511,"context_line":"        self.mock_update_node.assert_called_once_with(mock.ANY, mock.ANY)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F84U8%3D","line":508,"in_reply_to":"AAAATn%2F87PU%3D","updated":"2013-12-19 08:58:04.000000000","message":"Done","commit_id":"b7337b34c378ac00c4c9c4e2979def59ec4db3ee"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ec61b83af36e3d410c344b123d6fa8e6a1b9b319","unresolved":false,"context_lines":[{"line_number":467,"context_line":"        self.assertEqual(response.status_code, 400)"},{"line_number":468,"context_line":"        self.assertTrue(response.json[\u0027error_message\u0027])"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def test_set_node_reservation(self):"},{"line_number":471,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":472,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"},{"line_number":473,"context_line":"                                   \u0027op\u0027: \u0027replace\u0027}], expect_errors\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F8mOs%3D","line":470,"updated":"2013-12-24 15:38:30.000000000","message":"Maybe rename to test_set_node_reservation_not_allowed ?","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e01d6bdf2b5e489c9e765e7f084fc476ee1742ba","unresolved":false,"context_lines":[{"line_number":467,"context_line":"        self.assertEqual(response.status_code, 400)"},{"line_number":468,"context_line":"        self.assertTrue(response.json[\u0027error_message\u0027])"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def test_set_node_reservation(self):"},{"line_number":471,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node[\u0027uuid\u0027],"},{"line_number":472,"context_line":"                                   [{\u0027path\u0027: \u0027/reservation\u0027, \u0027value\u0027: \u0027fake\u0027,"},{"line_number":473,"context_line":"                                   \u0027op\u0027: \u0027replace\u0027}], expect_errors\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAT3%2F%2F%2FCc%3D","line":470,"in_reply_to":"AAAATn%2F8mOs%3D","updated":"2013-12-25 12:51:17.000000000","message":"Done","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ec61b83af36e3d410c344b123d6fa8e6a1b9b319","unresolved":false,"context_lines":[{"line_number":475,"context_line":"        self.assertEqual(response.status_code, 400)"},{"line_number":476,"context_line":"        self.assertTrue(response.json[\u0027error_message\u0027])"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    def test_clear_node_reservation_conductor_active(self):"},{"line_number":479,"context_line":"        with mock.patch.object(self.dbapi, \u0027get_active_driver_dict\u0027) as d_mock:"},{"line_number":480,"context_line":"            d_mock.return_value \u003d {\u0027drv1\u0027: set([\u0027fake-host\u0027, \u0027fake-host-2\u0027])}"},{"line_number":481,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F8mNg%3D","line":478,"updated":"2013-12-24 15:38:30.000000000","message":"test_clear_node_reservation_not_allowed_when_conductor_active\n\nI know it\u0027s long, but makes the test purpose clear","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e01d6bdf2b5e489c9e765e7f084fc476ee1742ba","unresolved":false,"context_lines":[{"line_number":475,"context_line":"        self.assertEqual(response.status_code, 400)"},{"line_number":476,"context_line":"        self.assertTrue(response.json[\u0027error_message\u0027])"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    def test_clear_node_reservation_conductor_active(self):"},{"line_number":479,"context_line":"        with mock.patch.object(self.dbapi, \u0027get_active_driver_dict\u0027) as d_mock:"},{"line_number":480,"context_line":"            d_mock.return_value \u003d {\u0027drv1\u0027: set([\u0027fake-host\u0027, \u0027fake-host-2\u0027])}"},{"line_number":481,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAT3%2F%2F%2FBs%3D","line":478,"in_reply_to":"AAAATn%2F8mNg%3D","updated":"2013-12-25 12:51:17.000000000","message":"Done","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"ec61b83af36e3d410c344b123d6fa8e6a1b9b319","unresolved":false,"context_lines":[{"line_number":490,"context_line":"            self.assertEqual(response.status_code, 400)"},{"line_number":491,"context_line":"            self.assertTrue(response.json[\u0027error_message\u0027])"},{"line_number":492,"context_line":""},{"line_number":493,"context_line":"    def test_clear_node_reservation_conductor_ok(self):"},{"line_number":494,"context_line":"        with mock.patch.object(self.dbapi, \u0027get_active_driver_dict\u0027) as d_mock:"},{"line_number":495,"context_line":"            d_mock.return_value \u003d {\u0027drv1\u0027: set([\u0027fake-host-2\u0027, \u0027fake-host-3\u0027])}"},{"line_number":496,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAATn%2F8mOA%3D","line":493,"updated":"2013-12-24 15:38:30.000000000","message":"I suggest just test_clear_node_reservation","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e01d6bdf2b5e489c9e765e7f084fc476ee1742ba","unresolved":false,"context_lines":[{"line_number":490,"context_line":"            self.assertEqual(response.status_code, 400)"},{"line_number":491,"context_line":"            self.assertTrue(response.json[\u0027error_message\u0027])"},{"line_number":492,"context_line":""},{"line_number":493,"context_line":"    def test_clear_node_reservation_conductor_ok(self):"},{"line_number":494,"context_line":"        with mock.patch.object(self.dbapi, \u0027get_active_driver_dict\u0027) as d_mock:"},{"line_number":495,"context_line":"            d_mock.return_value \u003d {\u0027drv1\u0027: set([\u0027fake-host-2\u0027, \u0027fake-host-3\u0027])}"},{"line_number":496,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAT3%2F%2F%2FC0%3D","line":493,"in_reply_to":"AAAATn%2F8mOA%3D","updated":"2013-12-25 12:51:17.000000000","message":"Done","commit_id":"7fde1aafc3259f907dc52b10ce447f4209c47413"}],"ironic/tests/db/test_conductor.py":[{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"e6f6bcdedfab296c47136af6f24480fb516ae39b","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        result \u003d self.dbapi.get_active_conductors_list(interval\u003d1)"},{"line_number":184,"context_line":"        self.assertEqual(sorted(hosts), sorted(result))"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    def test_get_active_conductors_list_with_old_conductor(self):"},{"line_number":187,"context_line":"        past \u003d datetime.datetime(2000, 1, 1, 0, 0)"},{"line_number":188,"context_line":"        present \u003d past + datetime.timedelta(minutes\u003d2)"},{"line_number":189,"context_line":"        d \u003d \u0027common-driver\u0027"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAUH%2F%2Bi4U%3D","line":186,"updated":"2014-01-24 15:21:02.000000000","message":"since you refactored get_active_driver_dict to split out common method, I think it\u0027s best to do the same with the unit tests here. \n\n- test the time-sensitive nature of _get_active_conductors in a few methods by itself\n- test the return data structure of get_active_driver_dict\n- test the return data structure of get_active_conductor_list","commit_id":"af001092e9f7dce8e72b4a8824aeb7f25c40ed4e"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"d33c4013efa16074f858d360a016dcaf1534a250","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        result \u003d self.dbapi.get_active_conductors_list(interval\u003d1)"},{"line_number":184,"context_line":"        self.assertEqual(sorted(hosts), sorted(result))"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    def test_get_active_conductors_list_with_old_conductor(self):"},{"line_number":187,"context_line":"        past \u003d datetime.datetime(2000, 1, 1, 0, 0)"},{"line_number":188,"context_line":"        present \u003d past + datetime.timedelta(minutes\u003d2)"},{"line_number":189,"context_line":"        d \u003d \u0027common-driver\u0027"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAUH%2F%2BFoU%3D","line":186,"in_reply_to":"AAAAUH%2F%2Bi4U%3D","updated":"2014-01-30 09:10:49.000000000","message":"Devananda, _get_active_conductors is \u0027private\u0027 dbapi methods, IMO it should\u0027nt be tested in public dbapi tests.","commit_id":"af001092e9f7dce8e72b4a8824aeb7f25c40ed4e"}],"ironic/tests/db/test_nodes.py":[{"author":{"_account_id":7491,"name":"Viktor Serhieiev","email":"viktor.sergeyev@gmail.com","username":"vsergeyev"},"change_message_id":"962c4a4fa34fd547fda1faa08fa28026dd2313ea","unresolved":false,"context_lines":[{"line_number":343,"context_line":"        # release reservation w/o tag"},{"line_number":344,"context_line":"        self.dbapi.release_nodes(None, [uuid])"},{"line_number":345,"context_line":"        res \u003d self.dbapi.get_node(uuid)"},{"line_number":346,"context_line":"        self.assertEqual(None, res[\u0027reservation\u0027])"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def test_force_release_reservation_notfound(self):"},{"line_number":349,"context_line":"        self.assertRaises(exception.NodeNotFound,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2Bnz0%3D","line":346,"updated":"2013-11-22 10:52:20.000000000","message":"nit: self.assertIsNone()","commit_id":"627eebdf145afd2662f20489fedd8de7b35677c2"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e421366ec2a1aba22e108562f89a833ed7d45feb","unresolved":false,"context_lines":[{"line_number":343,"context_line":"        # release reservation w/o tag"},{"line_number":344,"context_line":"        self.dbapi.release_nodes(None, [uuid])"},{"line_number":345,"context_line":"        res \u003d self.dbapi.get_node(uuid)"},{"line_number":346,"context_line":"        self.assertEqual(None, res[\u0027reservation\u0027])"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def test_force_release_reservation_notfound(self):"},{"line_number":349,"context_line":"        self.assertRaises(exception.NodeNotFound,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2Bm90%3D","line":346,"in_reply_to":"AAAATn%2F%2Bnz0%3D","updated":"2013-11-22 14:49:19.000000000","message":"Done","commit_id":"627eebdf145afd2662f20489fedd8de7b35677c2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"b0e97a29d447bd26920f13ae8526039bc749f2f3","unresolved":false,"context_lines":[{"line_number":362,"context_line":"        self.assertRaises(exception.NodeNotFound,"},{"line_number":363,"context_line":"                          self.dbapi.release_nodes, None,"},{"line_number":364,"context_line":"                          [\u002712345678-9999-0000-aaaa-123456789012\u0027])"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"    def test_get_associated_nodes(self):"},{"line_number":367,"context_line":"        (uuids, uuids_with_instance) \u003d self._create_associated_nodes()"},{"line_number":368,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAT3%2F%2F7Sc%3D","line":365,"updated":"2013-12-27 00:47:08.000000000","message":"No need to hardcode the uuid, just call utils.generate_uuid() to create one.","commit_id":"ec3a88ca3bf1ee5f5115d9e598852953208dd398"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"0f9ad2d746265ccd0266a602e83b5c597d700627","unresolved":false,"context_lines":[{"line_number":362,"context_line":"        self.assertRaises(exception.NodeNotFound,"},{"line_number":363,"context_line":"                          self.dbapi.release_nodes, None,"},{"line_number":364,"context_line":"                          [\u002712345678-9999-0000-aaaa-123456789012\u0027])"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"    def test_get_associated_nodes(self):"},{"line_number":367,"context_line":"        (uuids, uuids_with_instance) \u003d self._create_associated_nodes()"},{"line_number":368,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAT3%2F%2F1aQ%3D","line":365,"in_reply_to":"AAAAT3%2F%2F7Sc%3D","updated":"2013-12-30 14:41:14.000000000","message":"Done","commit_id":"ec3a88ca3bf1ee5f5115d9e598852953208dd398"}]}