)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"07fa5badab4cd1f340edc2947c0b8e5f755624c9","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently as a part of locking an instance, while we know who locked"},{"line_number":10,"context_line":"it we do not have a way of knowing “why” it was locked. This spec"},{"line_number":11,"context_line":"aims at addressing this question."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I8908a204432e8ac35f5d15f17b5a567d4732ad0e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fdfeff1_e3827e00","line":12,"updated":"2019-02-22 11:15:06.000000000","message":"nit: tag the blueprint.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"850d107bab19516dcbd66635a54612fa384ab11e","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently as a part of locking an instance, while we know who locked"},{"line_number":10,"context_line":"it we do not have a way of knowing “why” it was locked. This spec"},{"line_number":11,"context_line":"aims at addressing this question."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I8908a204432e8ac35f5d15f17b5a567d4732ad0e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5fc1f717_f549d531","line":12,"in_reply_to":"5fc1f717_7ceb9711","updated":"2019-03-22 10:52:28.000000000","message":"Done","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"850d107bab19516dcbd66635a54612fa384ab11e","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently as a part of locking an instance, while we know who locked"},{"line_number":10,"context_line":"it we do not have a way of knowing “why” it was locked. This spec"},{"line_number":11,"context_line":"aims at addressing this question."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I8908a204432e8ac35f5d15f17b5a567d4732ad0e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5fc1f717_15458115","line":12,"in_reply_to":"9fdfeff1_e3827e00","updated":"2019-03-22 10:52:28.000000000","message":"Done","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8be251729051c98a79153ab44eb2475b368f529e","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently as a part of locking an instance, while we know who locked"},{"line_number":10,"context_line":"it we do not have a way of knowing “why” it was locked. This spec"},{"line_number":11,"context_line":"aims at addressing this question."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I8908a204432e8ac35f5d15f17b5a567d4732ad0e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5fc1f717_7ceb9711","line":12,"in_reply_to":"9fdfeff1_e3827e00","updated":"2019-03-22 00:28:40.000000000","message":"Should also have the APIImpact tag.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"}],"specs/train/approved/add-locked-reason.rst":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new ``locked_reason`` column in the"},{"line_number":41,"context_line":"``nova.instances`` table which would be of type \"varchar\" allowing a maximum of"},{"line_number":42,"context_line":"255 characters. The corresponding ``locked_reason`` field will be added in the"},{"line_number":43,"context_line":"Instance object."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_f1ab3b72","line":42,"range":{"start_line":40,"start_character":0,"end_line":42,"end_character":15},"updated":"2019-03-21 17:14:02.000000000","message":"For the sake of argument, does this need to be in the instances table? Will anything need to query this, or filter instances based on this column? If it just metadata, then we could store it in the instance_system_metadata table which already allows values of 255. Then it\u0027s just a matter of clearing it from the system_metadata when the server is unlocked. I guess the argument could be made that the existing locked and locked_by columns are in the instances table, but it seems unnecessary that they are and that we would need to continue storing this type of metadata there.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new ``locked_reason`` column in the"},{"line_number":41,"context_line":"``nova.instances`` table which would be of type \"varchar\" allowing a maximum of"},{"line_number":42,"context_line":"255 characters. The corresponding ``locked_reason`` field will be added in the"},{"line_number":43,"context_line":"Instance object."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_bff24c99","line":42,"range":{"start_line":40,"start_character":0,"end_line":42,"end_character":15},"in_reply_to":"5fc1f717_1c7db36d","updated":"2019-03-22 10:44:19.000000000","message":"I do agree with the fact that locked_reason need not be specifically in the instances table, fyi I added it there only because the locked and locked_by reside there. \n\nIf its making it uglier to add more \"lock\" stuff into the instance table, should I first propose another spec to move the existing \"locked_by\" into the metadata table and then add the locked_reason column there? So that means we have the boolean in the instance object and more information in the metadata.\n\nNot sure it that is too much work though, it would be nice to have this asap (we have downstream inter-team/role locking use cases).","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"863fda99bc4c1d6ec5ef9ec3f13e2b0e3fc6e8aa","unresolved":false,"context_lines":[{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new ``locked_reason`` column in the"},{"line_number":41,"context_line":"``nova.instances`` table which would be of type \"varchar\" allowing a maximum of"},{"line_number":42,"context_line":"255 characters. The corresponding ``locked_reason`` field will be added in the"},{"line_number":43,"context_line":"Instance object."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_bd190289","line":42,"range":{"start_line":40,"start_character":0,"end_line":42,"end_character":15},"in_reply_to":"5fc1f717_4445ab14","updated":"2019-03-21 19:58:43.000000000","message":"Yeah, pretty much agree with Ed here: It may have been wrong to put locked_by in there in the first place, but now that it is having locked_by and locked_reason in two different places would be confusing and a bit painful.\n\nIf we want to predicate this work on a cleanup of the many tables involved in describing an instance, we could, but that kind of stop energy is likely to have us all in a funk.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fce020b7e16e43087079517a086db555f010a6f9","unresolved":false,"context_lines":[{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new ``locked_reason`` column in the"},{"line_number":41,"context_line":"``nova.instances`` table which would be of type \"varchar\" allowing a maximum of"},{"line_number":42,"context_line":"255 characters. The corresponding ``locked_reason`` field will be added in the"},{"line_number":43,"context_line":"Instance object."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_1c7db36d","line":42,"range":{"start_line":40,"start_character":0,"end_line":42,"end_character":15},"in_reply_to":"5fc1f717_bd190289","updated":"2019-03-22 00:27:13.000000000","message":"Sure, I\u0027m not -1 on the data model change proposed here, it\u0027s just a bit regrettable, like the POST action itself rather than just doing a simple PUT.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"796a163dd2375e4771d4942160c9cfb775c71a1b","unresolved":false,"context_lines":[{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new ``locked_reason`` column in the"},{"line_number":41,"context_line":"``nova.instances`` table which would be of type \"varchar\" allowing a maximum of"},{"line_number":42,"context_line":"255 characters. The corresponding ``locked_reason`` field will be added in the"},{"line_number":43,"context_line":"Instance object."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_4445ab14","line":42,"range":{"start_line":40,"start_character":0,"end_line":42,"end_character":15},"in_reply_to":"5fc1f717_f1ab3b72","updated":"2019-03-21 17:30:03.000000000","message":"It would be much more confusing to have locked_by in one place and locked_reason in another. As a first step, locked_reason should probably be added to the instances table. At a later date, if a refactoring is undertaken, they should all be moved together. You could probably dump the redundant boolean column at the same time.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new ``locked_reason`` column in the"},{"line_number":41,"context_line":"``nova.instances`` table which would be of type \"varchar\" allowing a maximum of"},{"line_number":42,"context_line":"255 characters. The corresponding ``locked_reason`` field will be added in the"},{"line_number":43,"context_line":"Instance object."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""},{"line_number":46,"context_line":"will get a new optional argument called ``reason`` by which the user can"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_b1dcd318","line":43,"range":{"start_line":42,"start_character":16,"end_line":43,"end_character":16},"updated":"2019-03-21 17:14:02.000000000","message":"If we want, we could still add an Instance.locked_reason field as an abstraction for where the data actually resides, if we decide to store it in system_metadata. Or we could just avoid all that and rely on the key being in system_metadata or it\u0027s not there for old instances (or no lock reason was given).","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Alternatives"},{"line_number":55,"context_line":"------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Lock can be made it’s `own resource`_ in which case we can add a new lock API"},{"line_number":58,"context_line":"which would look like ``GET servers/{server_id}/lock`` that can include the"},{"line_number":59,"context_line":"details like locked or not, locked_by, locked_reason and timestamp. But we"},{"line_number":60,"context_line":"already have a ``GET servers/{server_id}/os-instance-actions/{request_id}``"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_71e66beb","line":57,"range":{"start_line":57,"start_character":17,"end_line":57,"end_character":21},"updated":"2019-03-21 17:14:02.000000000","message":"its","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Alternatives"},{"line_number":55,"context_line":"------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Lock can be made it’s `own resource`_ in which case we can add a new lock API"},{"line_number":58,"context_line":"which would look like ``GET servers/{server_id}/lock`` that can include the"},{"line_number":59,"context_line":"details like locked or not, locked_by, locked_reason and timestamp. But we"},{"line_number":60,"context_line":"already have a ``GET servers/{server_id}/os-instance-actions/{request_id}``"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_327d336e","line":57,"range":{"start_line":57,"start_character":17,"end_line":57,"end_character":21},"in_reply_to":"5fc1f717_71e66beb","updated":"2019-03-22 10:44:19.000000000","message":"Done","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5b69a939aadac827c77e1059d9c3ef605624505e","unresolved":false,"context_lines":[{"line_number":75,"context_line":"REST API impact"},{"line_number":76,"context_line":"---------------"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"},{"line_number":79,"context_line":"get a new optional argument called ``reason`` which will accept the reason for"},{"line_number":80,"context_line":"locking the server and store this in the instances table in the database."},{"line_number":81,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_bcb25f48","line":78,"range":{"start_line":78,"start_character":16,"end_line":78,"end_character":52},"updated":"2019-03-22 00:26:26.000000000","message":"You know, I realize changing this now would be a lot more change than is probably wanted here, but it\u0027s really dumb that we added an entire POST action just to change a field on the server resource. Meaning, this could have / should have all just been done with PUT /servers/{server_id} from the beginning because that\u0027s really all this is doing, flipping a bit on the resource. The locked_by is sugar added by the role on the context making the request, it\u0027s not part of the request payload itself.\n\nSo anyway, if I could do this over again from scratch, I would have just done it as part of updating a server.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e925b99680739a8be41a83cc2bf1c5570041f5df","unresolved":false,"context_lines":[{"line_number":75,"context_line":"REST API impact"},{"line_number":76,"context_line":"---------------"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"},{"line_number":79,"context_line":"get a new optional argument called ``reason`` which will accept the reason for"},{"line_number":80,"context_line":"locking the server and store this in the instances table in the database."},{"line_number":81,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_d5ea99d6","line":78,"range":{"start_line":78,"start_character":16,"end_line":78,"end_character":52},"in_reply_to":"5fc1f717_bcb25f48","updated":"2019-03-22 11:05:20.000000000","message":"well I guess its between the fine line of thinking if \"lock\" is an action on the server versus if its just an update to the instance.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a62df2598ad56b83f69a0fede3f160ae0a4d1182","unresolved":false,"context_lines":[{"line_number":75,"context_line":"REST API impact"},{"line_number":76,"context_line":"---------------"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"},{"line_number":79,"context_line":"get a new optional argument called ``reason`` which will accept the reason for"},{"line_number":80,"context_line":"locking the server and store this in the instances table in the database."},{"line_number":81,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_ec5907fe","line":78,"range":{"start_line":78,"start_character":16,"end_line":78,"end_character":52},"in_reply_to":"5fc1f717_d5ea99d6","updated":"2019-03-22 15:44:20.000000000","message":"I don\u0027t know what the original reasoning was behind making it an action, maybe Dan remembers, but it definitely doesn\u0027t really do anything, it just flips a bit on the instance object. I guess it does send a notification now as well.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":82,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    {"},{"line_number":85,"context_line":"        \"lock\": null,"},{"line_number":86,"context_line":"        \"locked_reason\": \"foo-test\""},{"line_number":87,"context_line":"    }"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_918517d2","line":85,"range":{"start_line":85,"start_character":8,"end_line":85,"end_character":21},"updated":"2019-03-21 17:14:02.000000000","message":"This is weird semantically. Maybe we want to follow what was done for cold migration with a target host where you\u0027d have either:\n\n1. \"lock\": null - as today\n\n2. \"lock\": {\n       \"locked_reason\": \"because I am mad at belmiro\"\n   }\n\nhttps://developer.openstack.org/api-ref/compute/?expanded\u003dlock-server-lock-action-detail,migrate-server-migrate-action-detail#migrate-server-migrate-action","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":82,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    {"},{"line_number":85,"context_line":"        \"lock\": null,"},{"line_number":86,"context_line":"        \"locked_reason\": \"foo-test\""},{"line_number":87,"context_line":"    }"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_1564a1e4","line":85,"range":{"start_line":85,"start_character":8,"end_line":85,"end_character":21},"in_reply_to":"5fc1f717_5864ace5","updated":"2019-03-22 10:44:19.000000000","message":"I guess this means I would change my request body type to being an object which would either be NULL or from the \"2.xx\" have a body with the locked_reason if specified ?","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"236da8de18005cdfa63d9bf94db0b7078e6ff691","unresolved":false,"context_lines":[{"line_number":82,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    {"},{"line_number":85,"context_line":"        \"lock\": null,"},{"line_number":86,"context_line":"        \"locked_reason\": \"foo-test\""},{"line_number":87,"context_line":"    }"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_d5b93915","line":85,"range":{"start_line":85,"start_character":8,"end_line":85,"end_character":21},"in_reply_to":"5fc1f717_918517d2","updated":"2019-03-22 10:51:27.000000000","message":"\u003e 2. \"lock\": {\n \u003e \"locked_reason\": \"because I am mad at belmiro\"\n \u003e }\n \u003e \n\nheh what a nice reason to lock :)","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"863fda99bc4c1d6ec5ef9ec3f13e2b0e3fc6e8aa","unresolved":false,"context_lines":[{"line_number":82,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    {"},{"line_number":85,"context_line":"        \"lock\": null,"},{"line_number":86,"context_line":"        \"locked_reason\": \"foo-test\""},{"line_number":87,"context_line":"    }"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_5864ace5","line":85,"range":{"start_line":85,"start_character":8,"end_line":85,"end_character":21},"in_reply_to":"5fc1f717_918517d2","updated":"2019-03-21 19:58:43.000000000","message":"Do you mean \"lock\": null in old microversions, \"lock\": {\"locked_reason\": in new}\n\nOr actually allowing \"lock\": null as a different type in the new microversion?\n\nI think that could get messy. maybe the new microversion should require a version and clients can choose to provide a default?","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        \"locked_reason\": \"foo-test\""},{"line_number":87,"context_line":"    }"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"We plan to expose the ``locked_by`` and ``locked_reason`` information through"},{"line_number":90,"context_line":"``GET servers/{server_id}`` and ``GET /servers/detail`` REST APIs whose"},{"line_number":91,"context_line":"reponses will have a change in their \"locked\" key."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":94,"context_line":"conveys if the instance is locked or not based on if its true or false. The"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_d164ff4b","line":91,"range":{"start_line":89,"start_character":0,"end_line":91,"end_character":50},"updated":"2019-03-21 17:14:02.000000000","message":"What about PUT (update) and rebuild responses which also return a server response body using the same view builder?","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        \"locked_reason\": \"foo-test\""},{"line_number":87,"context_line":"    }"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"We plan to expose the ``locked_by`` and ``locked_reason`` information through"},{"line_number":90,"context_line":"``GET servers/{server_id}`` and ``GET /servers/detail`` REST APIs whose"},{"line_number":91,"context_line":"reponses will have a change in their \"locked\" key."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":94,"context_line":"conveys if the instance is locked or not based on if its true or false. The"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_32cbf3a5","line":91,"range":{"start_line":89,"start_character":0,"end_line":91,"end_character":50},"in_reply_to":"5fc1f717_d164ff4b","updated":"2019-03-22 10:44:19.000000000","message":"ack, I\u0027ll add these API\u0027s too.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":93,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":94,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":95,"context_line":"type of this key will be changed to a dict so that it can include the"},{"line_number":96,"context_line":"``locked_by`` and ``locked_reason`` informations in additon to the status."},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"A sample JSON response would look like this for a locked server::"},{"line_number":99,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_31554351","line":96,"range":{"start_line":96,"start_character":52,"end_line":96,"end_character":59},"updated":"2019-03-21 17:14:02.000000000","message":"addition","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":93,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":94,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":95,"context_line":"type of this key will be changed to a dict so that it can include the"},{"line_number":96,"context_line":"``locked_by`` and ``locked_reason`` informations in additon to the status."},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"A sample JSON response would look like this for a locked server::"},{"line_number":99,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_5280ff46","line":96,"range":{"start_line":96,"start_character":52,"end_line":96,"end_character":59},"in_reply_to":"5fc1f717_31554351","updated":"2019-03-22 10:44:19.000000000","message":"Done","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                ."},{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_719e6b08","line":110,"range":{"start_line":110,"start_character":20,"end_line":110,"end_character":35},"updated":"2019-03-21 17:14:02.000000000","message":"\"status\" seems weird here but I\u0027m not sure I have a better alternative.\n\nOne idea, which is maybe gross, would be to have locked be a boolean (if false) or a dict (if true) so if you get a dict, it\u0027s got locked detailed, else it\u0027s false meaning it\u0027s not locked.\n\nMaybe need input from API SIG minded people like edleafe and/or cdent.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5b69a939aadac827c77e1059d9c3ef605624505e","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                ."},{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_1ca4d307","line":110,"range":{"start_line":110,"start_character":20,"end_line":110,"end_character":35},"in_reply_to":"5fc1f717_18e1442b","updated":"2019-03-22 00:26:26.000000000","message":"\u003e empty dict for not locked, dict with by and reason (potentially\n \u003e with empty string) if locked.\n \u003e \n \u003e This allows code which is simply checking if a server is locked to\n \u003e be the same whether they are using the old or new microversion: if\n \u003e servers[\u0027locked\u0027]: do_something_with_my_locked_server\n \u003e \n \u003e Whatever the new structure, status is redundant and can probably\n \u003e go.\n\nYup I like that and it was my 2nd alternative:\n\n\u003e Alternatively, just return {} if it\u0027s not locked, and if it is locked, you get the dict with locked_by and locked_reason keys in it.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                ."},{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_92866765","line":110,"range":{"start_line":110,"start_character":20,"end_line":110,"end_character":35},"in_reply_to":"5fc1f717_1ca4d307","updated":"2019-03-22 10:44:19.000000000","message":"+1 for having \"{}\", see my comment on L138 for why I had decided to add the status in the first place.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b52309f06560f5f968f88b7299c5e9f1a56ac341","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                ."},{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_84f5d3ef","line":110,"range":{"start_line":110,"start_character":20,"end_line":110,"end_character":35},"in_reply_to":"5fc1f717_719e6b08","updated":"2019-03-21 17:21:44.000000000","message":"\u003e One idea, which is maybe gross, would be to have locked be a boolean (if false) or a dict (if true) so if you get a dict, it\u0027s got locked detailed, else it\u0027s false meaning it\u0027s not locked.\n\nAlternatively, just return {} if it\u0027s not locked, and if it is locked, you get the dict with locked_by and locked_reason keys in it.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"796a163dd2375e4771d4942160c9cfb775c71a1b","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                ."},{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_e476173f","line":110,"range":{"start_line":110,"start_character":20,"end_line":110,"end_character":35},"in_reply_to":"5fc1f717_719e6b08","updated":"2019-03-21 17:30:03.000000000","message":"I\u0027m kinda yuck about having the type be different depending on state. Either go with the \u0027status\u0027 value, or drop that and just return _by and _reason. If those are empty, then the instance isn\u0027t locked.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"670a04fcfeee7f41a2eb57305559e187cb3807d3","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                ."},{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_a7b94dcc","line":110,"range":{"start_line":110,"start_character":20,"end_line":110,"end_character":35},"in_reply_to":"5fc1f717_92866765","updated":"2019-03-28 17:09:56.000000000","message":"IMO, missing fields in response either that is in nested fields or direct is complicated for the clients to use those attributes. \n\nIf we always return the locked_by and lock_reason always and keep null value if the server is not locked.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"863fda99bc4c1d6ec5ef9ec3f13e2b0e3fc6e8aa","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                ."},{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_18e1442b","line":110,"range":{"start_line":110,"start_character":20,"end_line":110,"end_character":35},"in_reply_to":"5fc1f717_e476173f","updated":"2019-03-21 19:58:43.000000000","message":"empty dict for not locked, dict with by and reason (potentially with empty string) if locked.\n\nThis allows code which is simply checking if a server is locked to be the same whether they are using the old or new microversion: if servers[\u0027locked\u0027]: do_something_with_my_locked_server\n\nWhatever the new structure, status is redundant and can probably go.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"},{"line_number":114,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_11dfe7d6","line":111,"range":{"start_line":111,"start_character":33,"end_line":111,"end_character":52},"updated":"2019-03-21 17:14:02.000000000","message":"Ew I guess we do store this as an enum in the database but it will be weird with any future scope type changes we made, i.e. you could have project admin scope or system admin scope, so which would it be? Seems like this is leaking weird information about the policy engine.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8012bd089b195ff11c685428bb81b9a6b356a21b","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"},{"line_number":114,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_6c599758","line":111,"range":{"start_line":111,"start_character":33,"end_line":111,"end_character":52},"in_reply_to":"5fc1f717_0c5fb372","updated":"2019-03-22 15:56:55.000000000","message":"\u003e We should allow for a value here that indicates that it\u0027s neither,\n \u003e or more complicated. If we later want to allow locking by a user\n \u003e based on a role, we\u0027ll need to store the user-id. How about we add\n \u003e an \"other\" or \"user\" here, so that if we do that in the future, we\n \u003e don\u0027t have to fret about how to handle this api when a non-owner\n \u003e non-admin has locked this instance?\n\nI\u0027m not sure if we need to really, the Instance.locked_by is just a string field. It\u0027s an enum in the DB, but what goes into the object and will be spit back out of the API here is just that string. So if at some point we start storing a user_id in there or something, the API should just work, yeah?","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"},{"line_number":114,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_1264b7e4","line":111,"range":{"start_line":111,"start_character":33,"end_line":111,"end_character":52},"in_reply_to":"5fc1f717_11dfe7d6","updated":"2019-03-22 10:44:19.000000000","message":"\u003e Ew I guess we do store this as an enum in the database \n\n:( yea\n\n \u003e but it will\n \u003e be weird with any future scope type changes we made, i.e. you could\n \u003e have project admin scope or system admin scope, so which would it\n \u003e be? Seems like this is leaking weird information about the policy\n \u003e engine.\n\nI am not sure what to do though since this has been there for quite some time now","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"502950a30c71857c987b863f23addeaf5a68c5e7","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"},{"line_number":114,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_0c5fb372","line":111,"range":{"start_line":111,"start_character":33,"end_line":111,"end_character":52},"in_reply_to":"5fc1f717_1264b7e4","updated":"2019-03-22 15:49:43.000000000","message":"We should allow for a value here that indicates that it\u0027s neither, or more complicated. If we later want to allow locking by a user based on a role, we\u0027ll need to store the user-id. How about we add an \"other\" or \"user\" here, so that if we do that in the future, we don\u0027t have to fret about how to handle this api when a non-owner non-admin has locked this instance?","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0e177936169fbea8fea8eb7dce2672bd30f2f8be","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                ."},{"line_number":109,"context_line":"                \"locked\": {"},{"line_number":110,"context_line":"                    \"status\": true,"},{"line_number":111,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":112,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":113,"context_line":"                },"},{"line_number":114,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_0f81e54a","line":111,"range":{"start_line":111,"start_character":33,"end_line":111,"end_character":52},"in_reply_to":"5fc1f717_6c599758","updated":"2019-03-22 16:01:40.000000000","message":"I mean here in the spec, it looks to me like we\u0027re saying it will be one or the other, so let\u0027s call it out as a string so that it doesn\u0027t get encoded as an enum in anyone\u0027s client or expectations.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                ."},{"line_number":136,"context_line":"                ."},{"line_number":137,"context_line":"                ."},{"line_number":138,"context_line":"                \"locked\": {"},{"line_number":139,"context_line":"                    \"status\": false,"},{"line_number":140,"context_line":"                    \"locked_by\": null,"},{"line_number":141,"context_line":"                    \"locked_reason\": null,"},{"line_number":142,"context_line":"                },"},{"line_number":143,"context_line":"                \"name\": \"surya-probes-001\","},{"line_number":144,"context_line":"                \"OS-EXT-SRV-ATTR:launch_index\": 0,"},{"line_number":145,"context_line":"                \"created\": \"2018-06-29T15:07:29Z\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_118da7d2","line":142,"range":{"start_line":138,"start_character":16,"end_line":142,"end_character":18},"updated":"2019-03-21 17:14:02.000000000","message":"Yeah this seems weird to me as noted above, but let\u0027s see what others think.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"796a163dd2375e4771d4942160c9cfb775c71a1b","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                ."},{"line_number":136,"context_line":"                ."},{"line_number":137,"context_line":"                ."},{"line_number":138,"context_line":"                \"locked\": {"},{"line_number":139,"context_line":"                    \"status\": false,"},{"line_number":140,"context_line":"                    \"locked_by\": null,"},{"line_number":141,"context_line":"                    \"locked_reason\": null,"},{"line_number":142,"context_line":"                },"},{"line_number":143,"context_line":"                \"name\": \"surya-probes-001\","},{"line_number":144,"context_line":"                \"OS-EXT-SRV-ATTR:launch_index\": 0,"},{"line_number":145,"context_line":"                \"created\": \"2018-06-29T15:07:29Z\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_84b4f3d2","line":142,"range":{"start_line":138,"start_character":16,"end_line":142,"end_character":18},"in_reply_to":"5fc1f717_118da7d2","updated":"2019-03-21 17:30:03.000000000","message":"The boolean seems redundant to me. IOW, it can never not be false when either locked_by or locked_reason is not null, can it?","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"6810aa816a79de67d2c7db8cd3473fbd6799f97f","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                ."},{"line_number":136,"context_line":"                ."},{"line_number":137,"context_line":"                ."},{"line_number":138,"context_line":"                \"locked\": {"},{"line_number":139,"context_line":"                    \"status\": false,"},{"line_number":140,"context_line":"                    \"locked_by\": null,"},{"line_number":141,"context_line":"                    \"locked_reason\": null,"},{"line_number":142,"context_line":"                },"},{"line_number":143,"context_line":"                \"name\": \"surya-probes-001\","},{"line_number":144,"context_line":"                \"OS-EXT-SRV-ATTR:launch_index\": 0,"},{"line_number":145,"context_line":"                \"created\": \"2018-06-29T15:07:29Z\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_d5985976","line":142,"range":{"start_line":138,"start_character":16,"end_line":142,"end_character":18},"in_reply_to":"5fc1f717_5fa1e845","updated":"2019-03-22 11:08:54.000000000","message":"oh yea now I remember the other reason was to not have the different types based on if its locked or not :)","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                ."},{"line_number":136,"context_line":"                ."},{"line_number":137,"context_line":"                ."},{"line_number":138,"context_line":"                \"locked\": {"},{"line_number":139,"context_line":"                    \"status\": false,"},{"line_number":140,"context_line":"                    \"locked_by\": null,"},{"line_number":141,"context_line":"                    \"locked_reason\": null,"},{"line_number":142,"context_line":"                },"},{"line_number":143,"context_line":"                \"name\": \"surya-probes-001\","},{"line_number":144,"context_line":"                \"OS-EXT-SRV-ATTR:launch_index\": 0,"},{"line_number":145,"context_line":"                \"created\": \"2018-06-29T15:07:29Z\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_5fa1e845","line":142,"range":{"start_line":138,"start_character":16,"end_line":142,"end_character":18},"in_reply_to":"5fc1f717_84b4f3d2","updated":"2019-03-22 10:44:19.000000000","message":"yea I did \"not want to add status at all\" this way, I guess the reason why I added the status was to keep it saner like what we have today where people expect a True or False response for the lock based on the \"locked\" boolean column and all of a sudden it changes to an empty dict, but I guess that\u0027s the whole point of microversion based incremental api changes. I am more than happy to disolve this status key. Besides just having the locked boolean doesn\u0027t mean we need to necessarily expose it always.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":165,"context_line":"Notifications impact"},{"line_number":166,"context_line":"--------------------"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"None."},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"Other end user impact"},{"line_number":171,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_51ac8f34","line":168,"updated":"2019-03-21 17:14:02.000000000","message":"I assume the InstancePayload has locked in it already yeah? So there should be a change here as well.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":165,"context_line":"Notifications impact"},{"line_number":166,"context_line":"--------------------"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"None."},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"Other end user impact"},{"line_number":171,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_df287898","line":168,"in_reply_to":"5fc1f717_51ac8f34","updated":"2019-03-22 10:44:19.000000000","message":"I must admit that I have been completely ignorant of the \"nova/notifications\" repo. Thanks for pointing this out Matt","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"6547a62b0da66511e0269f7650c7fd5f32955882","unresolved":false,"context_lines":[{"line_number":165,"context_line":"Notifications impact"},{"line_number":166,"context_line":"--------------------"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"None."},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"Other end user impact"},{"line_number":171,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_ebf7b2d3","line":168,"in_reply_to":"5fc1f717_abb76a3a","updated":"2019-03-22 13:10:32.000000000","message":"I mean is there any reason for us to change the payload https://github.com/openstack/nova/blob/master/doc/notification_samples/instance-lock.json ? / https://github.com/openstack/nova/blob/b33fa1c054ba4b7d4e789aa51250ad5c8325da2d/nova/notifications/objects/instance.py#L110 ?","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"802ac702488c212dcbd7fd356ed9e313dd31733b","unresolved":false,"context_lines":[{"line_number":165,"context_line":"Notifications impact"},{"line_number":166,"context_line":"--------------------"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"None."},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"Other end user impact"},{"line_number":171,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_abb76a3a","line":168,"in_reply_to":"5fc1f717_df287898","updated":"2019-03-22 13:00:43.000000000","message":"@mriedem: yea just went through the nova action notifications. InstancePayload has \"locked\" field which is the boolean, not sure why I should change that ? I mean we don\u0027t necessarily need the locked_reason/locked_by in the notification right ?","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"cdf8834c4e9067d5015f40991e6a85d6be4d22f7","unresolved":false,"context_lines":[{"line_number":165,"context_line":"Notifications impact"},{"line_number":166,"context_line":"--------------------"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"None."},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"Other end user impact"},{"line_number":171,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_cb719646","line":168,"in_reply_to":"5fc1f717_ebf7b2d3","updated":"2019-03-22 13:17:06.000000000","message":"It would be nice to add the locked_reason and the locked_by to the notification payload.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":170,"context_line":"Other end user impact"},{"line_number":171,"context_line":"---------------------"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"In order to be able to provide a reason and then see this when asking for a"},{"line_number":174,"context_line":"server show, the python-novaclient will be updated:"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"* to add the new optional parameter ``--reason`` for"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_71b74b88","line":173,"updated":"2019-03-21 17:14:02.000000000","message":"python-openstackclient should be called out here because we want to close the feature parity gaps in OSC, not widen them.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":170,"context_line":"Other end user impact"},{"line_number":171,"context_line":"---------------------"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"In order to be able to provide a reason and then see this when asking for a"},{"line_number":174,"context_line":"server show, the python-novaclient will be updated:"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"* to add the new optional parameter ``--reason`` for"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_5f5ec876","line":173,"in_reply_to":"5fc1f717_71b74b88","updated":"2019-03-22 10:44:19.000000000","message":"Done","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":216,"context_line":"#. Add a new column ``locked_reason`` to nova.instances table."},{"line_number":217,"context_line":"#. Add a new field ``locked_reason`` to Instance object."},{"line_number":218,"context_line":"#. Handle ``nova lock`` on receiving an optional \"reason\" parameter."},{"line_number":219,"context_line":"#. Expose the ``locked_by`` and ``locked_reason`` through ``nova show``."},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Dependencies"},{"line_number":222,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_b1305309","line":219,"range":{"start_line":219,"start_character":58,"end_line":219,"end_character":71},"updated":"2019-03-21 17:14:02.000000000","message":"I really wish you wouldn\u0027t reference nova CLI in specs when describing API features, because the nova CLI is not the only way that people access the API, so just stick to discussing details about the REST APIs, the client side stuff can be assumed. If you\u0027re just calling out specific work for python-novaclient, sure that\u0027s fine, but then also call it out for OSC.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":216,"context_line":"#. Add a new column ``locked_reason`` to nova.instances table."},{"line_number":217,"context_line":"#. Add a new field ``locked_reason`` to Instance object."},{"line_number":218,"context_line":"#. Handle ``nova lock`` on receiving an optional \"reason\" parameter."},{"line_number":219,"context_line":"#. Expose the ``locked_by`` and ``locked_reason`` through ``nova show``."},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Dependencies"},{"line_number":222,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_9f64304b","line":219,"range":{"start_line":219,"start_character":58,"end_line":219,"end_character":71},"in_reply_to":"5fc1f717_b1305309","updated":"2019-03-22 10:44:19.000000000","message":"sorry I will keep this in mind and change this to talk in terms of REST APIs.","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c64bf1176dfb3a62d69e6ef3b319664662afc3f","unresolved":false,"context_lines":[{"line_number":234,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"Update the description of the Compute API reference with regards to the"},{"line_number":237,"context_line":"``nova show`` and ``nova lock`` commands."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"References"},{"line_number":240,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_713aebe7","line":237,"updated":"2019-03-21 17:14:02.000000000","message":"same","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1265d44f038d492f6331605e8e5df3aad8c27821","unresolved":false,"context_lines":[{"line_number":234,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"Update the description of the Compute API reference with regards to the"},{"line_number":237,"context_line":"``nova show`` and ``nova lock`` commands."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"References"},{"line_number":240,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc1f717_3f5bbc87","line":237,"in_reply_to":"5fc1f717_713aebe7","updated":"2019-03-22 10:44:19.000000000","message":"Done","commit_id":"181bdc871f143d015dea60ba1ba226b22873ec7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a62df2598ad56b83f69a0fede3f160ae0a4d1182","unresolved":false,"context_lines":[{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new ``locked_reason`` column in the"},{"line_number":41,"context_line":"``nova.instances`` table which would be of type \"varchar\" allowing a maximum of"},{"line_number":42,"context_line":"255 characters. The corresponding ``locked_reason`` field will be added in the"},{"line_number":43,"context_line":"Instance object."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_ec40270a","line":42,"range":{"start_line":40,"start_character":0,"end_line":42,"end_character":15},"updated":"2019-03-22 15:44:20.000000000","message":"I know we talked about this in PS1, but after discussing with Dan on IRC he came to the same conclusion I did which is we should just use system_metadata which is the right place for this information anyway:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-03-22.log.html#t2019-03-22T15:19:35\n\nI\u0027m not asking for a big data migration to move locked and locked_by, but let\u0027s use the right place for this from the start. The Instance object will still have the locked_reason field but will populate it from system_metadata instead (which is another good reason why we can leverage Instance as an abstraction over the location of the actual data).","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"8bd79231172134dc67deb2a22225e89872f3a201","unresolved":false,"context_lines":[{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new ``locked_reason`` column in the"},{"line_number":41,"context_line":"``nova.instances`` table which would be of type \"varchar\" allowing a maximum of"},{"line_number":42,"context_line":"255 characters. The corresponding ``locked_reason`` field will be added in the"},{"line_number":43,"context_line":"Instance object."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_7dfdc338","line":42,"range":{"start_line":40,"start_character":0,"end_line":42,"end_character":15},"in_reply_to":"5fc1f717_ec40270a","updated":"2019-03-25 09:37:08.000000000","message":"Done","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":47,"context_line":"specify the reason for locking the instance."},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"The plan is to expose ``locked_by`` and ``locked_reason``  information through"},{"line_number":50,"context_line":"``GET servers/{server_id}`` and ``GET /servers/detail`` requests by modifying"},{"line_number":51,"context_line":"the existing ``locked`` status information in the response. See the"},{"line_number":52,"context_line":"`REST API impact`_ section for more details on how this would be done."},{"line_number":53,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_b20d9a3f","line":50,"range":{"start_line":50,"start_character":0,"end_line":50,"end_character":54},"updated":"2019-03-22 17:32:15.000000000","message":"for PUT and Rebuild also for consistent response schema for all server representation. Though it is weird to expose lock information in rebuild because we do not allow lock instance to rebuild so those informations will null always. \n\nBut we already exposed \u0027lock\u0027 in rebuild response for consistency. IMO we should add all new information for PUT as well as in rebuild response also.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":50,"context_line":"``GET servers/{server_id}`` and ``GET /servers/detail`` requests by modifying"},{"line_number":51,"context_line":"the existing ``locked`` status information in the response. See the"},{"line_number":52,"context_line":"`REST API impact`_ section for more details on how this would be done."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"Alternatives"},{"line_number":55,"context_line":"------------"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_bd3ecb26","line":53,"range":{"start_line":53,"start_character":0,"end_line":53,"end_character":0},"updated":"2019-03-22 17:32:15.000000000","message":"we should mention that on \u0027unlock\u0027 we need to clear the lock_reason information in DB otherwise GET server will keep fetching the old information and show in response.\n\nLike other lock information is being cleared by unlock action - https://github.com/openstack/nova/blob/d7c8924b0c22b86068724724462291a810161795/nova/compute/api.py#L4042","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e053a8790233b41b8ebb6549e777cec6b3150db","unresolved":false,"context_lines":[{"line_number":50,"context_line":"``GET servers/{server_id}`` and ``GET /servers/detail`` requests by modifying"},{"line_number":51,"context_line":"the existing ``locked`` status information in the response. See the"},{"line_number":52,"context_line":"`REST API impact`_ section for more details on how this would be done."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"Alternatives"},{"line_number":55,"context_line":"------------"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_f5df8737","line":53,"range":{"start_line":53,"start_character":0,"end_line":53,"end_character":0},"in_reply_to":"5fc1f717_bd3ecb26","updated":"2019-03-22 20:17:18.000000000","message":"I think that\u0027s kind of an obvious implementation detail.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a62df2598ad56b83f69a0fede3f160ae0a4d1182","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Alternatives"},{"line_number":55,"context_line":"------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Lock can be made its `own resource`_ in which case we can add a new lock API"},{"line_number":58,"context_line":"which would look like ``GET servers/{server_id}/lock`` that can include the"},{"line_number":59,"context_line":"details like locked or not, locked_by, locked_reason and timestamp. But we"},{"line_number":60,"context_line":"already have a ``GET servers/{server_id}/os-instance-actions/{request_id}``"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_cc452b17","line":57,"updated":"2019-03-22 15:44:20.000000000","message":"Gross no, but a more reasonable alternative as I said before would be to just change the PUT /servers/{server_id} API for this rather than even have a lock action (just deprecate the lock action since a POST to change a field on a resource is dumb).\n\nI guess one thing we get with the separate lock action now is a dedicated notification, so that\u0027s probably useful on its own.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e053a8790233b41b8ebb6549e777cec6b3150db","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Alternatives"},{"line_number":55,"context_line":"------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Lock can be made its `own resource`_ in which case we can add a new lock API"},{"line_number":58,"context_line":"which would look like ``GET servers/{server_id}/lock`` that can include the"},{"line_number":59,"context_line":"details like locked or not, locked_by, locked_reason and timestamp. But we"},{"line_number":60,"context_line":"already have a ``GET servers/{server_id}/os-instance-actions/{request_id}``"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_35d66f29","line":57,"in_reply_to":"5fc1f717_1dcfdfb0","updated":"2019-03-22 20:17:18.000000000","message":"\u003e but how about adding the ability to update the lock reason in PUT\n \u003e API? will there be case where admin/owner want to update the\n \u003e lock_reason after initial lock.\n\nUgh...this is why it should have been a PUT /servers/{server_id} to begin with....\n\nIf we also support modifying the locked_reason in PUT then we have to add in logic in update() to see if locked is True and 400 (or something) otherwise. This seemed like such a simple thing and is getting complicated.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5a8e11ba4988875053d30bcd15fc53d2ac7262d8","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Alternatives"},{"line_number":55,"context_line":"------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Lock can be made its `own resource`_ in which case we can add a new lock API"},{"line_number":58,"context_line":"which would look like ``GET servers/{server_id}/lock`` that can include the"},{"line_number":59,"context_line":"details like locked or not, locked_by, locked_reason and timestamp. But we"},{"line_number":60,"context_line":"already have a ``GET servers/{server_id}/os-instance-actions/{request_id}``"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_f5b467d3","line":57,"in_reply_to":"5fc1f717_35d66f29","updated":"2019-03-22 20:35:16.000000000","message":"yeah little complicated. let\u0027s leave the PUT thing.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"8bd79231172134dc67deb2a22225e89872f3a201","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Alternatives"},{"line_number":55,"context_line":"------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Lock can be made its `own resource`_ in which case we can add a new lock API"},{"line_number":58,"context_line":"which would look like ``GET servers/{server_id}/lock`` that can include the"},{"line_number":59,"context_line":"details like locked or not, locked_by, locked_reason and timestamp. But we"},{"line_number":60,"context_line":"already have a ``GET servers/{server_id}/os-instance-actions/{request_id}``"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_fd6c5309","line":57,"in_reply_to":"5fc1f717_cc452b17","updated":"2019-03-25 09:37:08.000000000","message":"Done","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":54,"context_line":"Alternatives"},{"line_number":55,"context_line":"------------"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Lock can be made its `own resource`_ in which case we can add a new lock API"},{"line_number":58,"context_line":"which would look like ``GET servers/{server_id}/lock`` that can include the"},{"line_number":59,"context_line":"details like locked or not, locked_by, locked_reason and timestamp. But we"},{"line_number":60,"context_line":"already have a ``GET servers/{server_id}/os-instance-actions/{request_id}``"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_1dcfdfb0","line":57,"in_reply_to":"5fc1f717_cc452b17","updated":"2019-03-22 17:32:15.000000000","message":"but how about adding the ability to update the lock reason in PUT API? will there be case where admin/owner want to update the lock_reason after initial lock.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Lock can be made its `own resource`_ in which case we can add a new lock API"},{"line_number":58,"context_line":"which would look like ``GET servers/{server_id}/lock`` that can include the"},{"line_number":59,"context_line":"details like locked or not, locked_by, locked_reason and timestamp. But we"},{"line_number":60,"context_line":"already have a ``GET servers/{server_id}/os-instance-actions/{request_id}``"},{"line_number":61,"context_line":"request API to get the details like timestamp. So it does not make sense to add"},{"line_number":62,"context_line":"a whole new API for just retrieving the locked_by and locked_reason"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_7d6383dd","line":59,"range":{"start_line":59,"start_character":56,"end_line":59,"end_character":66},"updated":"2019-03-22 17:32:15.000000000","message":"yeah, i remember adding as NOTE for my old spec -https://review.openstack.org/#/c/206864/1/specs/liberty/approved/add-locking-information-in-server-get-response.rst","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":57,"context_line":"Lock can be made its `own resource`_ in which case we can add a new lock API"},{"line_number":58,"context_line":"which would look like ``GET servers/{server_id}/lock`` that can include the"},{"line_number":59,"context_line":"details like locked or not, locked_by, locked_reason and timestamp. But we"},{"line_number":60,"context_line":"already have a ``GET servers/{server_id}/os-instance-actions/{request_id}``"},{"line_number":61,"context_line":"request API to get the details like timestamp. So it does not make sense to add"},{"line_number":62,"context_line":"a whole new API for just retrieving the locked_by and locked_reason"},{"line_number":63,"context_line":"informations."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Data model impact"},{"line_number":66,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_fda4530a","line":63,"range":{"start_line":60,"start_character":15,"end_line":63,"end_character":13},"updated":"2019-03-22 17:32:15.000000000","message":"+1, make sense.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31c6ea101354d8a8ffe972ca934896d53fa2dcaa","unresolved":false,"context_lines":[{"line_number":72,"context_line":"Also, the ``Instance`` object will need to gain a new field called"},{"line_number":73,"context_line":"``locked_reason``."},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"REST API impact"},{"line_number":76,"context_line":"---------------"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_d5aa8b83","line":75,"updated":"2019-03-22 20:20:22.000000000","message":"Throwing this out there, but while we\u0027re talking about returning a response body now, do we want to take this opportunity to fix the response code either way? It\u0027s a 202 now but should be a 204:\n\nhttps://github.com/openstack/nova/blob/c58bd5854ddcf447ae5c1421e63d1e959adb782c/nova/api/openstack/compute/lock_server.py#L27\n\nIf we start returning a response body, it\u0027d become a 200.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5a8e11ba4988875053d30bcd15fc53d2ac7262d8","unresolved":false,"context_lines":[{"line_number":72,"context_line":"Also, the ``Instance`` object will need to gain a new field called"},{"line_number":73,"context_line":"``locked_reason``."},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"REST API impact"},{"line_number":76,"context_line":"---------------"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_55c53b3b","line":75,"in_reply_to":"5fc1f717_d5aa8b83","updated":"2019-03-22 20:35:16.000000000","message":"+1 to fix it to 200.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    \"lock\": {"},{"line_number":85,"context_line":"       \"locked_reason\": \"because I am mad at belmiro\""},{"line_number":86,"context_line":"    }"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The request body would either be a \"null\" object in case the reason is not"},{"line_number":89,"context_line":"specified or it will have a \"locked_reason\" field in the object (possible from"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_5d3487fb","line":86,"range":{"start_line":84,"start_character":0,"end_line":86,"end_character":5},"updated":"2019-03-22 17:32:15.000000000","message":"may be we should show schema here along with the example. something like\n\n    \"lock\": {\n        \u0027oneOf\u0027: [{\u0027type\u0027: \u0027string\u0027, \u0027minLength\u0027: 1, \u0027maxLength\u0027: 255,}, \u0027pattern\u0027: \u0027locked_reason\u0027},\n                  {\u0027type\u0027: \u0027null\u0027}]\n    }\n\nor with enum\n\n    \"lock\": {\n        \u0027oneOf\u0027: [{\u0027type\u0027: \u0027string\u0027, \u0027minLength\u0027: 1, \u0027maxLength\u0027: 255,}, \u0027enum\u0027: [\u0027locked_reason\u0027]},\n                  {\u0027type\u0027: \u0027null\u0027}]\n    }","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":85,"context_line":"       \"locked_reason\": \"because I am mad at belmiro\""},{"line_number":86,"context_line":"    }"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The request body would either be a \"null\" object in case the reason is not"},{"line_number":89,"context_line":"specified or it will have a \"locked_reason\" field in the object (possible from"},{"line_number":90,"context_line":"the new microversion)."},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"We plan to expose the ``locked_by`` and ``locked_reason`` information through"},{"line_number":93,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_3dd3bb3a","line":90,"range":{"start_line":88,"start_character":0,"end_line":90,"end_character":22},"updated":"2019-03-22 17:32:15.000000000","message":"+1 for making \u0027locked_reason\u0027 as an optional field in request body because not all the time user want to set reason.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a62df2598ad56b83f69a0fede3f160ae0a4d1182","unresolved":false,"context_lines":[{"line_number":89,"context_line":"specified or it will have a \"locked_reason\" field in the object (possible from"},{"line_number":90,"context_line":"the new microversion)."},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"We plan to expose the ``locked_by`` and ``locked_reason`` information through"},{"line_number":93,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":94,"context_line":"``POST /servers/{server_id}/action``  where the action is rebuild and"},{"line_number":95,"context_line":"``GET servers/{server_id}`` REST APIs whose reponses will have a change in"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_6c02f7b6","line":92,"updated":"2019-03-22 15:44:20.000000000","message":"You missed PUT /servers/{server_id} response in here as well.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"280a1da3f2e4b20e5d517a3e079e5642ae85d818","unresolved":false,"context_lines":[{"line_number":89,"context_line":"specified or it will have a \"locked_reason\" field in the object (possible from"},{"line_number":90,"context_line":"the new microversion)."},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"We plan to expose the ``locked_by`` and ``locked_reason`` information through"},{"line_number":93,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":94,"context_line":"``POST /servers/{server_id}/action``  where the action is rebuild and"},{"line_number":95,"context_line":"``GET servers/{server_id}`` REST APIs whose reponses will have a change in"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_8c43a3af","line":92,"in_reply_to":"5fc1f717_6c02f7b6","updated":"2019-03-22 16:18:52.000000000","message":"uh-oh I wrote the GET one twice instead.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":89,"context_line":"specified or it will have a \"locked_reason\" field in the object (possible from"},{"line_number":90,"context_line":"the new microversion)."},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"We plan to expose the ``locked_by`` and ``locked_reason`` information through"},{"line_number":93,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":94,"context_line":"``POST /servers/{server_id}/action``  where the action is rebuild and"},{"line_number":95,"context_line":"``GET servers/{server_id}`` REST APIs whose reponses will have a change in"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_fddc5347","line":92,"in_reply_to":"5fc1f717_8c43a3af","updated":"2019-03-22 17:32:15.000000000","message":"+ Rebuild also.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":93,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":94,"context_line":"``POST /servers/{server_id}/action``  where the action is rebuild and"},{"line_number":95,"context_line":"``GET servers/{server_id}`` REST APIs whose reponses will have a change in"},{"line_number":96,"context_line":"their \"locked\" key."},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":99,"context_line":"conveys if the instance is locked or not based on if its true or false. The"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_1d5fdf72","line":96,"range":{"start_line":96,"start_character":0,"end_line":96,"end_character":19},"updated":"2019-03-22 17:32:15.000000000","message":"we should return the \u0027lock_reason\u0027 information in lock API response also like we do for service disable reason. \n\nThat way user requesting lock with lock_reason can see the same in response and confirmed it is set. It can be a mandatory field with an empty string if not set in the request. \n\nhttps://developer.openstack.org/api-ref/compute/#disable-scheduling-for-a-compute-service-and-log-disabled-reason","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0f3cf18536e1ad338d7486bc2ec0132d9162ff94","unresolved":false,"context_lines":[{"line_number":93,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":94,"context_line":"``POST /servers/{server_id}/action``  where the action is rebuild and"},{"line_number":95,"context_line":"``GET servers/{server_id}`` REST APIs whose reponses will have a change in"},{"line_number":96,"context_line":"their \"locked\" key."},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":99,"context_line":"conveys if the instance is locked or not based on if its true or false. The"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_f5430712","line":96,"range":{"start_line":96,"start_character":0,"end_line":96,"end_character":19},"in_reply_to":"5fc1f717_1d5fdf72","updated":"2019-03-22 20:09:17.000000000","message":"Note that version of the service disable API is deprecated in favor of a generic PUT /os-services/{service_id} which returns the normal GET response body. So if we\u0027re going to start returning a response body from this API, it could be the same as we use for PUT /servers/{server_id} because as noted in PS1, this is essentially just a PUT on the server but we decided to implement it as a POST initially for some weird reason.\n\nAnyway, I don\u0027t have super strong feelings about this, but I just wish we were more consistent in the compute API like how the placement API is, i.e. GETs and PUTs are mirrored and just fields on the resource being updated and shown, not subsets and extra weird stuff that\u0027s not part of the GET response. Returning just the locked_reason would be like a PATCH which is more like what this is, but it\u0027s not a PATCH.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5a8e11ba4988875053d30bcd15fc53d2ac7262d8","unresolved":false,"context_lines":[{"line_number":93,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":94,"context_line":"``POST /servers/{server_id}/action``  where the action is rebuild and"},{"line_number":95,"context_line":"``GET servers/{server_id}`` REST APIs whose reponses will have a change in"},{"line_number":96,"context_line":"their \"locked\" key."},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":99,"context_line":"conveys if the instance is locked or not based on if its true or false. The"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_7571570c","line":96,"range":{"start_line":96,"start_character":0,"end_line":96,"end_character":19},"in_reply_to":"5fc1f717_f5430712","updated":"2019-03-22 20:35:16.000000000","message":"agree but with lock_reason setting in POST /servers/{server_id}/action {lock}, this API is more than just shooting action on the server. \n\nI am not against on idea of doing the same API improvement for server/actions also like you did for os-service API.  It will be good if we merge the few of the server actions in PUT server.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5eeb95407294aaa5e8696aac6901d15c9dbc3303","unresolved":false,"context_lines":[{"line_number":98,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":99,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":100,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":101,"context_line":"and ``locked_reason`` informations. So an empty dict would mean the instance"},{"line_number":102,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":103,"context_line":"would mean its locked."},{"line_number":104,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_2f9b81c9","line":101,"range":{"start_line":101,"start_character":22,"end_line":101,"end_character":34},"updated":"2019-03-22 16:00:39.000000000","message":"information","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":113,"context_line":"                ."},{"line_number":114,"context_line":"                ."},{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                \"locked\": {"},{"line_number":117,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":118,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":119,"context_line":"                },"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_185dade8","line":116,"range":{"start_line":116,"start_character":15,"end_line":116,"end_character":27},"updated":"2019-03-22 17:32:15.000000000","message":"can we have the schema also for this field to know the possible value types along with example?\n\nlike this?\n\n\"locked\": {\n                    \"locked_by\": \u0027oneOf\u0027: [{\u0027type\u0027: \u0027string\u0027, \u0027format\u0027: \u0027uuid\u0027},\n                                          {\u0027type\u0027: \u0027null\u0027}],\n                    \"locked_reason\": \u0027oneOf\u0027: [{\u0027type\u0027: \u0027string\u0027, \u0027format\u0027: \u0027uuid\u0027},\n                                          {\u0027type\u0027: \u0027null\u0027}],\n                },","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a62df2598ad56b83f69a0fede3f160ae0a4d1182","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                ."},{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                \"locked\": {"},{"line_number":117,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":118,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":119,"context_line":"                },"},{"line_number":120,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_cc1c4b15","line":117,"range":{"start_line":117,"start_character":20,"end_line":117,"end_character":52},"updated":"2019-03-22 15:44:20.000000000","message":"I would like to talk with Lance about this.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                ."},{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                \"locked\": {"},{"line_number":117,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":118,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":119,"context_line":"                },"},{"line_number":120,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_3d8adb1a","line":117,"range":{"start_line":117,"start_character":20,"end_line":117,"end_character":52},"in_reply_to":"5fc1f717_0f86c5dc","updated":"2019-03-22 17:32:15.000000000","message":"yeah, we should expose the uuid but will it be project_id or user_id. We only check and set locked_by based on project_id. \n\nWhen we will add scope type which will be \u0027system\u0027 and \u0027project\u0027 in lock case (current proposal in my PoC [1]), we would not distinguish about what \u0027admin\u0027 means (system or project).\n\n+1 to add the string type here. api-ref can tell this is just string with UUID type. \n\nOne question- In response, we are going to expose user-id right? even though it is not stored in DB but we can still fetch that via context based on locked_by DB value. context.project_id if instance.locked_by \u003d\u003d \u0027owner\u0027 else DO not know how to fetch admin uuid from context if it is not admin request ?\n\n\n[1] https://review.openstack.org/#/c/645452/2/nova/policies/lock_server.py","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"04b77f88b8f33c2fe5799c298b1693bf30a59822","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                ."},{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                \"locked\": {"},{"line_number":117,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":118,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":119,"context_line":"                },"},{"line_number":120,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_928899c6","line":117,"range":{"start_line":117,"start_character":20,"end_line":117,"end_character":52},"in_reply_to":"5fc1f717_230cfa39","updated":"2019-03-22 19:45:12.000000000","message":"other updates on this: instead of exposing the proejct_id, locked_by\u003downer|other is better way seems\n\n- http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-03-22.log.html","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"55b9d27d6edfdacb3aa67bb4d5e03a03ae71383b","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                ."},{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                \"locked\": {"},{"line_number":117,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":118,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":119,"context_line":"                },"},{"line_number":120,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_230cfa39","line":117,"range":{"start_line":117,"start_character":20,"end_line":117,"end_character":52},"in_reply_to":"5fc1f717_3d8adb1a","updated":"2019-03-22 17:51:25.000000000","message":"as per discussion on IRC[1] (adding the link as tssurya was offiline that time), we can do the DB change also to store the project_id in locked_by column by adding the field \u0027id\u0027 in current enum. Then API only expose the project_id in reasponse. \n\nThis is the only API we are going to expose the string like \u0027admin\u0027, \u0027owner\u0027 where \u0027admin\u0027 means will be two different things after scope_type.\n\n[1] http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-03-22.log.html#t2019-03-22T17:34:44","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0232aaa62ce72f3b1d6aa079ea01f6461b693fba","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                ."},{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                \"locked\": {"},{"line_number":117,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":118,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":119,"context_line":"                },"},{"line_number":120,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_d50e0be4","line":117,"range":{"start_line":117,"start_character":20,"end_line":117,"end_character":52},"in_reply_to":"5fc1f717_928899c6","updated":"2019-03-22 20:11:49.000000000","message":"Umm, ignore everything gmann just said above - the summary is return locked_by as either \"owner\" or \"other\" where other can be \u0027admin\u0027 for what we have today, or something else in the future. The idea is \u0027other\u0027 is an indication that the user might not be able to unlock it, but they\u0027ll find out when they try and get a 403 if they aren\u0027t allowed.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"84b0dd28cfd74747d64b470e34c29aeab30d5f1d","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                ."},{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                \"locked\": {"},{"line_number":117,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":118,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":119,"context_line":"                },"},{"line_number":120,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_efa4990a","line":117,"range":{"start_line":117,"start_character":20,"end_line":117,"end_character":52},"in_reply_to":"5fc1f717_cc1c4b15","updated":"2019-03-22 16:00:46.000000000","message":"As Dan mentioned in PS1, let\u0027s not restrict this to \u0027admin\u0027 or \u0027owner\u0027 in the spec, in case we store user_id in the future or something, just say this is a string.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5a8e11ba4988875053d30bcd15fc53d2ac7262d8","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                ."},{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                \"locked\": {"},{"line_number":117,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":118,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":119,"context_line":"                },"},{"line_number":120,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_95d083f9","line":117,"range":{"start_line":117,"start_character":20,"end_line":117,"end_character":52},"in_reply_to":"5fc1f717_d50e0be4","updated":"2019-03-22 20:35:16.000000000","message":"yup. that\u0027s what i wrote in my last comment :) to gor for locked_by\u003downer|other","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"280a1da3f2e4b20e5d517a3e079e5642ae85d818","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                ."},{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                \"locked\": {"},{"line_number":117,"context_line":"                    \"locked_by\": \u0027admin\u0027 or \u0027owner\u0027,"},{"line_number":118,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":119,"context_line":"                },"},{"line_number":120,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_0f86c5dc","line":117,"range":{"start_line":117,"start_character":20,"end_line":117,"end_character":52},"in_reply_to":"5fc1f717_efa4990a","updated":"2019-03-22 16:18:52.000000000","message":"ack","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a62df2598ad56b83f69a0fede3f160ae0a4d1182","unresolved":false,"context_lines":[{"line_number":200,"context_line":"* to accommodate the parsing of the new dict type \"locked\" key in the server"},{"line_number":201,"context_line":"  response for ``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":202,"context_line":"  ``POST /servers/{server_id}/action``  where the action is rebuild and"},{"line_number":203,"context_line":"  ``GET servers/{server_id}`` REST APIs from the new microversion."},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"Performance Impact"},{"line_number":206,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_ccc68b4a","line":203,"range":{"start_line":203,"start_character":4,"end_line":203,"end_character":7},"updated":"2019-03-22 15:44:20.000000000","message":"PUT","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a62df2598ad56b83f69a0fede3f160ae0a4d1182","unresolved":false,"context_lines":[{"line_number":242,"context_line":"#. Handle ``POST /servers/{server_id}/action`` on receiving an optional"},{"line_number":243,"context_line":"   \"reason\" parameter."},{"line_number":244,"context_line":"#. Expose the ``locked_by`` and ``locked_reason`` through"},{"line_number":245,"context_line":"   ``GET servers/{server_id}``."},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"Dependencies"},{"line_number":248,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_4ce05bcd","line":245,"updated":"2019-03-22 15:44:20.000000000","message":"...and PUT and POST (rebuild) responses.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"20395753d81b4df244f4c63f54fd555072841d6a","unresolved":false,"context_lines":[{"line_number":254,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"Unit and functional tests for verifying the functionality."},{"line_number":257,"context_line":""},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"Documentation Impact"},{"line_number":260,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_3d8f3b24","line":257,"range":{"start_line":257,"start_character":0,"end_line":257,"end_character":0},"updated":"2019-03-22 17:32:15.000000000","message":"you can add tempest schema test also because this is changing the response schema. I can push changes for that.","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"8bd79231172134dc67deb2a22225e89872f3a201","unresolved":false,"context_lines":[{"line_number":254,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"Unit and functional tests for verifying the functionality."},{"line_number":257,"context_line":""},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"Documentation Impact"},{"line_number":260,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5fc1f717_eb478076","line":257,"range":{"start_line":257,"start_character":0,"end_line":257,"end_character":0},"in_reply_to":"5fc1f717_3d8f3b24","updated":"2019-03-25 09:37:08.000000000","message":"Done","commit_id":"2b1c9884116c187196e2ff8731aa435684327533"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8bc7e711491b4c681bc13d0fa52dd0e34761360","unresolved":false,"context_lines":[{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new ``locked_reason`` field to the Instance object."},{"line_number":41,"context_line":"The corresponding data for the locked_reason will be stored in the"},{"line_number":42,"context_line":"``nova.instance_system_metadata`` table as a key - value of type \"varchar\""},{"line_number":43,"context_line":"allowing a maximum of 255 characters. The value will be NULL at the time of"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_752ee08c","line":40,"updated":"2019-03-26 20:24:32.000000000","message":"I don\u0027t think we actually need a new field on the Instance object, it\u0027s just known system_metadata. I mean you could add it if you cared, but it\u0027s not really required, i.e. you could do what I suggested below. Alternatively, the Instance object can have a property method on it which retrieves the value from system_metadata as if there were a specific field. Ask Dan for his opinion.","commit_id":"f78cea6179447995e199909d2a9d03e33d42457e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"64e06053803ba990fe0f013d8124c49052b0bfcf","unresolved":false,"context_lines":[{"line_number":41,"context_line":"The corresponding data for the locked_reason will be stored in the"},{"line_number":42,"context_line":"``nova.instance_system_metadata`` table as a key - value of type \"varchar\""},{"line_number":43,"context_line":"allowing a maximum of 255 characters. The value will be NULL at the time of"},{"line_number":44,"context_line":"creation. For the existing instances which will not have this system metadata"},{"line_number":45,"context_line":"we will create it on the fly while locking."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""},{"line_number":48,"context_line":"will get a new optional argument called ``reason`` by which the user can"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_0e5c59eb","line":45,"range":{"start_line":44,"start_character":10,"end_line":45,"end_character":43},"updated":"2019-03-25 10:12:11.000000000","message":"is this how its done usually when adding new metadata?\n\nOh shoot no, if we are masking this in the instance.locked_reason field, I guess I need to do a migration for the existing ones...","commit_id":"f78cea6179447995e199909d2a9d03e33d42457e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"ff4bba1c3d1b7cfebfa3509a37cb752fcc31505a","unresolved":false,"context_lines":[{"line_number":41,"context_line":"The corresponding data for the locked_reason will be stored in the"},{"line_number":42,"context_line":"``nova.instance_system_metadata`` table as a key - value of type \"varchar\""},{"line_number":43,"context_line":"allowing a maximum of 255 characters. The value will be NULL at the time of"},{"line_number":44,"context_line":"creation. For the existing instances which will not have this system metadata"},{"line_number":45,"context_line":"we will create it on the fly while locking."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""},{"line_number":48,"context_line":"will get a new optional argument called ``reason`` by which the user can"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_2e327511","line":45,"range":{"start_line":44,"start_character":10,"end_line":45,"end_character":43},"in_reply_to":"5fc1f717_0e5c59eb","updated":"2019-03-25 10:32:27.000000000","message":"hmm, maybe its just easier to access it via Instance.system_metadata like other keys instead of having a locked_reason field ? but then it would be confusing considering we have the locked_by and locked fields.. like we discussed earlier.\n\nI am just thinking if its worth adding a new migration here, since its actually a very petty \"patch\" change.","commit_id":"f78cea6179447995e199909d2a9d03e33d42457e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ae82e396f6f09d155a3e89a6770ca9712aabbebc","unresolved":false,"context_lines":[{"line_number":41,"context_line":"The corresponding data for the locked_reason will be stored in the"},{"line_number":42,"context_line":"``nova.instance_system_metadata`` table as a key - value of type \"varchar\""},{"line_number":43,"context_line":"allowing a maximum of 255 characters. The value will be NULL at the time of"},{"line_number":44,"context_line":"creation. For the existing instances which will not have this system metadata"},{"line_number":45,"context_line":"we will create it on the fly while locking."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""},{"line_number":48,"context_line":"will get a new optional argument called ``reason`` by which the user can"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_d506b41c","line":45,"range":{"start_line":44,"start_character":10,"end_line":45,"end_character":43},"in_reply_to":"5fc1f717_2e327511","updated":"2019-03-26 20:22:46.000000000","message":"I\u0027m not sure what you\u0027re talking about. :) You don\u0027t need a schema migration. Existing instances won\u0027t have the \"locked_reason\" system_metadata entry. When locking with the new code, you just set the locked_reason system_metadata in the instance and save it:\n\ninstance.system_metadata[\u0027locked_reason\u0027] \u003d \u0027boohoo\u0027\ninstance.save()\n\nWhen returning the results, you just do this in the view builder:\n\n{\n   ...\n   \u0027locked_reason\u0027: instance.system_metadata.get(\u0027locked_reason\u0027)\n   ...\n}","commit_id":"f78cea6179447995e199909d2a9d03e33d42457e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"63a48389dd2364d4cefd562863d9f29910a2bae4","unresolved":false,"context_lines":[{"line_number":41,"context_line":"The corresponding data for the locked_reason will be stored in the"},{"line_number":42,"context_line":"``nova.instance_system_metadata`` table as a key - value of type \"varchar\""},{"line_number":43,"context_line":"allowing a maximum of 255 characters. The value will be NULL at the time of"},{"line_number":44,"context_line":"creation. For the existing instances which will not have this system metadata"},{"line_number":45,"context_line":"we will create it on the fly while locking."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""},{"line_number":48,"context_line":"will get a new optional argument called ``reason`` by which the user can"}],"source_content_type":"text/x-rst","patch_set":4,"id":"5fc1f717_f7ce4b2b","line":45,"range":{"start_line":44,"start_character":10,"end_line":45,"end_character":43},"in_reply_to":"5fc1f717_d506b41c","updated":"2019-03-27 09:37:33.000000000","message":"ah cool, thanks for clarifying. I got confused with the comments in the previous patchset regarding the new field.\n\nIf its just the system_metadata yea its fine.","commit_id":"f78cea6179447995e199909d2a9d03e33d42457e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b9859c016cba98eb01bf46b75a1f884c02dd0d12","unresolved":false,"context_lines":[{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new system metadata called ``locked_reason`` to the"},{"line_number":41,"context_line":"instance such that the data for the locked_reason will be stored in the"},{"line_number":42,"context_line":"``nova.instance_system_metadata`` table as a key - value of type \"varchar\""},{"line_number":43,"context_line":"allowing a maximum of 255 characters. This metadata key will not exist at the"},{"line_number":44,"context_line":"time of creation. For the existing instances which will not have this system"},{"line_number":45,"context_line":"metadata we will create it on the fly while locking."},{"line_number":46,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_c9992b6d","line":43,"range":{"start_line":40,"start_character":0,"end_line":43,"end_character":37},"updated":"2019-03-27 14:18:05.000000000","message":"This makes it sound like you\u0027re adding more than you are. The system_metadata is effectively just a dict, and you can store (generally) whatever you want in there, within the well-known constraints. So I would just collapse this to \"This spec proposes to store a locked_reason item in an instance\u0027s system_metadata.\"","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"73366b42de145c5129f405149d39337328ae5173","unresolved":false,"context_lines":[{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"This spec proposes to add a new system metadata called ``locked_reason`` to the"},{"line_number":41,"context_line":"instance such that the data for the locked_reason will be stored in the"},{"line_number":42,"context_line":"``nova.instance_system_metadata`` table as a key - value of type \"varchar\""},{"line_number":43,"context_line":"allowing a maximum of 255 characters. This metadata key will not exist at the"},{"line_number":44,"context_line":"time of creation. For the existing instances which will not have this system"},{"line_number":45,"context_line":"metadata we will create it on the fly while locking."},{"line_number":46,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_7ad68e51","line":43,"range":{"start_line":40,"start_character":0,"end_line":43,"end_character":37},"in_reply_to":"5fc1f717_c9992b6d","updated":"2019-03-29 10:07:30.000000000","message":"Done","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b9859c016cba98eb01bf46b75a1f884c02dd0d12","unresolved":false,"context_lines":[{"line_number":42,"context_line":"``nova.instance_system_metadata`` table as a key - value of type \"varchar\""},{"line_number":43,"context_line":"allowing a maximum of 255 characters. This metadata key will not exist at the"},{"line_number":44,"context_line":"time of creation. For the existing instances which will not have this system"},{"line_number":45,"context_line":"metadata we will create it on the fly while locking."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""},{"line_number":48,"context_line":"will get a new optional argument called ``reason`` by which the user can"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_69cd9764","line":45,"updated":"2019-03-27 14:18:05.000000000","message":"This too is just how system_metadata works. We should add something there when it\u0027s relevant and delete it when it\u0027s not. Thus, we should always add the item when locking and delete when unlocking.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"73366b42de145c5129f405149d39337328ae5173","unresolved":false,"context_lines":[{"line_number":42,"context_line":"``nova.instance_system_metadata`` table as a key - value of type \"varchar\""},{"line_number":43,"context_line":"allowing a maximum of 255 characters. This metadata key will not exist at the"},{"line_number":44,"context_line":"time of creation. For the existing instances which will not have this system"},{"line_number":45,"context_line":"metadata we will create it on the fly while locking."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The request for ``POST /servers/{server_id}/action`` where action is \"lock\""},{"line_number":48,"context_line":"will get a new optional argument called ``reason`` by which the user can"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_bade3667","line":45,"in_reply_to":"5fc1f717_69cd9764","updated":"2019-03-29 10:07:30.000000000","message":"Done","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b9859c016cba98eb01bf46b75a1f884c02dd0d12","unresolved":false,"context_lines":[{"line_number":48,"context_line":"will get a new optional argument called ``reason`` by which the user can"},{"line_number":49,"context_line":"specify the reason for locking the instance. If the reason is specified, it"},{"line_number":50,"context_line":"will create the instance_system_metadata value for that instance which will be"},{"line_number":51,"context_line":"cleared upon unlocking the instance and refilled if locked again."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"The plan is to expose ``locked_by`` and ``locked_reason``  information through"},{"line_number":54,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_e9de670f","line":51,"range":{"start_line":51,"start_character":0,"end_line":51,"end_character":7},"updated":"2019-03-27 14:18:05.000000000","message":"Deleted, for clarity.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b9859c016cba98eb01bf46b75a1f884c02dd0d12","unresolved":false,"context_lines":[{"line_number":48,"context_line":"will get a new optional argument called ``reason`` by which the user can"},{"line_number":49,"context_line":"specify the reason for locking the instance. If the reason is specified, it"},{"line_number":50,"context_line":"will create the instance_system_metadata value for that instance which will be"},{"line_number":51,"context_line":"cleared upon unlocking the instance and refilled if locked again."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"The plan is to expose ``locked_by`` and ``locked_reason``  information through"},{"line_number":54,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_c9e7ebe2","line":51,"range":{"start_line":51,"start_character":40,"end_line":51,"end_character":48},"updated":"2019-03-27 14:18:05.000000000","message":"Remove this I think.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"73366b42de145c5129f405149d39337328ae5173","unresolved":false,"context_lines":[{"line_number":48,"context_line":"will get a new optional argument called ``reason`` by which the user can"},{"line_number":49,"context_line":"specify the reason for locking the instance. If the reason is specified, it"},{"line_number":50,"context_line":"will create the instance_system_metadata value for that instance which will be"},{"line_number":51,"context_line":"cleared upon unlocking the instance and refilled if locked again."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"The plan is to expose ``locked_by`` and ``locked_reason``  information through"},{"line_number":54,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_9ae33a34","line":51,"range":{"start_line":51,"start_character":0,"end_line":51,"end_character":7},"in_reply_to":"5fc1f717_e9de670f","updated":"2019-03-29 10:07:30.000000000","message":"Done","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b9859c016cba98eb01bf46b75a1f884c02dd0d12","unresolved":false,"context_lines":[{"line_number":75,"context_line":"Data model impact"},{"line_number":76,"context_line":"-----------------"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"No DB schema change (other than adding the new instance system metadata"},{"line_number":79,"context_line":"key-value) or data migrations will be needed for this change."},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"REST API impact"},{"line_number":82,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_89f6c38e","line":79,"range":{"start_line":78,"start_character":19,"end_line":79,"end_character":61},"updated":"2019-03-27 14:18:05.000000000","message":"I would nix this. There\u0027s no data model impact.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"73366b42de145c5129f405149d39337328ae5173","unresolved":false,"context_lines":[{"line_number":75,"context_line":"Data model impact"},{"line_number":76,"context_line":"-----------------"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"No DB schema change (other than adding the new instance system metadata"},{"line_number":79,"context_line":"key-value) or data migrations will be needed for this change."},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"REST API impact"},{"line_number":82,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_da4b0228","line":79,"range":{"start_line":78,"start_character":19,"end_line":79,"end_character":61},"in_reply_to":"5fc1f717_89f6c38e","updated":"2019-03-29 10:07:30.000000000","message":"Done","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b9859c016cba98eb01bf46b75a1f884c02dd0d12","unresolved":false,"context_lines":[{"line_number":88,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    \"lock\": {"},{"line_number":91,"context_line":"       \"locked_reason\": \"because I am mad at belmiro\""},{"line_number":92,"context_line":"    }"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The request body would either be a \"null\" object in case the reason is not"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_e92c2703","line":91,"updated":"2019-03-27 14:18:05.000000000","message":"Nice :)","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"0ec777e462dcd41c9e8e84f07a7062fa0c585a85","unresolved":false,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The request body would either be a \"null\" object in case the reason is not"},{"line_number":95,"context_line":"specified or it will have a \"locked_reason\" field in the object (possible from"},{"line_number":96,"context_line":"the new microversion)."},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"We plan to expose the ``locked_by`` and ``locked_reason`` information through"},{"line_number":99,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_9fdd8742","line":96,"updated":"2019-03-28 07:03:49.000000000","message":"I guess we just reuse the schema of server\u0027s description.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b9859c016cba98eb01bf46b75a1f884c02dd0d12","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_0967334f","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"updated":"2019-03-27 14:18:05.000000000","message":"FWIW, I think this is a bad plan. It\u0027s just change that every client will have to digest when it really doesn\u0027t matter. Microversions allow us to be subtractive in our changes when necessary, but it just means that someone is going to need to have different code in their client for the old and new format, regardless of whether or not they care about the richer lock info.\n\nIMHO, we should keep locked\u003dBOOL, and add locked_by and locked_reason as peers.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"0ec777e462dcd41c9e8e84f07a7062fa0c585a85","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_3fe813e0","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"in_reply_to":"5fc1f717_0967334f","updated":"2019-03-28 07:03:49.000000000","message":"+1","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"39968abd6f5bf277281c6e230da0fc3b0263764e","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_cd77e59f","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"in_reply_to":"5fc1f717_3fe813e0","updated":"2019-03-28 09:02:58.000000000","message":"by \"as peers\" you mean as new keys in the response ? and not make it nested like I have proposed ? one of the reasons why I didn\u0027t do that was because most of the time those individual keys would be NULL.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a38f4ce4fe681977fd9ed64f872dafae0e30d7b3","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_b3de60d8","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"in_reply_to":"5fc1f717_6c7b4a7a","updated":"2019-03-29 14:07:25.000000000","message":"There was discussion about this earlier:\n\nhttps://review.openstack.org/#/c/638629/1/specs/train/approved/add-locked-reason.rst@110\n\nThe type changes from boolean to object (dict) but client side it should be fine if you\u0027re just doing something like:\n\nif server.locked:\n   ... locked logic\n\nBecause if the server is locked, the dict would be non-empty, but if it\u0027s unlocked the dict would be empty, and False-y equivalent, so behavior-wise it\u0027s a nice compromise. I also don\u0027t like adding more new sibling fields to the response.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"25241420b3e74b6b25de0a25bebee2d347dc08ae","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_ba3e1189","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"in_reply_to":"5fc1f717_76d46648","updated":"2019-04-01 17:39:23.000000000","message":"Another thing with making the new fields flat with locked, will we support filtering and sorting on locked_by and locked_reason? Because people will likely expect to be able to filter and sort on parameters in the server response body, except for things that are nested objects like bdms. There is a whitelist for valid sort/query parameters in the API code. Filtering on locked_reason would be dumb, but I could see people wanting to filter on locked_by.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c2cbb7ccd929414d79032c15a95df556a70b06aa","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_76d46648","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"in_reply_to":"5fc1f717_b3de60d8","updated":"2019-03-29 14:27:39.000000000","message":"That works for python but not many other languages, both because they might not be so loose with types, but also because an empty dict is not falsey in all languages. I think having a field called \"locked\" that has a value (which is not False or None) that means \"not locked\" is very confusing. Yes, in Python that duck types to not locked, but I think that is a bad assumption to make.\n\nAlso, as I\u0027ve been saying since the old says of the v3 fight, I don\u0027t think changes like this which purely serve to make nova developers feel like things are cleaner are worth breaking the api (and it is a break IMHO), regardless of whether or not heavy duck typing in one language might make it less of a thing or not.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"123f36c5214d248f75f96dfb4a90faeebcc16936","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_8592d657","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"in_reply_to":"5fc1f717_ba3e1189","updated":"2019-04-01 19:09:59.000000000","message":"+1, it is good idea to allow sort/filter by locked_by.\n\nI found that we already have locked_by field as valid sort key as well as filter key [1]. As we do not expose that field to user yet, having that in sort/filter whitelist was not usable. This is something we might have missed while preparing the whitelist. \n\n\n[1] https://github.com/openstack/nova/blob/a37b16610de0c09ea99b4e8a731b82e5bc1bd229/nova/api/openstack/compute/schemas/servers.py#L585\n\nhttps://github.com/openstack/nova/blob/a37b16610de0c09ea99b4e8a731b82e5bc1bd229/nova/api/openstack/compute/schemas/servers.py#L541","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"c5294a6cf4ad0f2de1ff27a45de1659de00f8195","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_ee5ff272","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"in_reply_to":"5fc1f717_ba3e1189","updated":"2019-04-02 07:38:06.000000000","message":"\u003e Another thing with making the new fields flat with locked,\n\nwait, so are we all on board with making the keys flat then ?\n\n\u003e will we support filtering and sorting on locked_by and locked_reason?\n \u003e Because people will likely expect to be able to filter and sort on\n \u003e parameters in the server response body, except for things that are\n \u003e nested objects like bdms. There is a whitelist for valid sort/query\n \u003e parameters in the API code. Filtering on locked_reason would be\n \u003e dumb, but I could see people wanting to filter on locked_by.\n\nsure we could add this to the whitelist like gmann suggests.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"8d94510926cb10bfce5829e1ba916efb74fcdf6d","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_5f103250","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"in_reply_to":"5fc1f717_ba3e1189","updated":"2019-04-02 09:37:57.000000000","message":"\u003e Filtering on locked_reason would be\n \u003e dumb, but I could see people wanting to filter on locked_by.\n\noh btw, we do have use_cases for filtering based on reason rather than locked_by because we don\u0027t necessarily expose which type of admin locked it while our automation job mostly will be programmed to have specific reasons under specific situations. I can understand why it would be dumb, but if we allow locked_by filtering and are going to make these two keys flat in the response, maybe we can also allow filtering based on locked_reason (like we have \"name\" regex filtering).","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"2e82839a6da5414d110126c7a7bce6b671661db9","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_6c7b4a7a","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"in_reply_to":"5fc1f717_cd77e59f","updated":"2019-03-28 16:19:56.000000000","message":"\u003e by \"as peers\" you mean as new keys in the response ? and not make\n \u003e it nested like I have proposed ? one of the reasons why I didn\u0027t do\n \u003e that was because most of the time those individual keys would be\n \u003e NULL.\n\nand for redundancy reasons.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"230c7fa21b5f2fb326666b23d69493daf09e1006","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Currently the response contains the \"locked\" key which is of type boolean that"},{"line_number":105,"context_line":"conveys if the instance is locked or not based on if its true or false. The"},{"line_number":106,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":107,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":108,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":109,"context_line":"would mean its locked."}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_31deff6c","line":106,"range":{"start_line":106,"start_character":0,"end_line":106,"end_character":33},"in_reply_to":"5fc1f717_cd77e59f","updated":"2019-03-28 15:54:14.000000000","message":"optional fields in API response is not good thing. we should always return the fields in response with some appropriate value or null. \n\n@Dan, are you suggesting something like this:\n\n                \"locked\": {\n                    \"locked\": \u003cboolean\u003e\n                    \"locked_by\": \u0027owner\u0027 or \u0027other\u0027,\n                    \"locked_reason\": \"foo-test\",\n                },\n\nor allowing \u0027locked_by\u0027 as null is enough ? which can convey that server is locked or not ?\n                \"locked\": {\n                    \"locked_by\": \u0027owner\u0027 or \u0027other\u0027 (string or null),\n                    \"locked_reason\": \"foo-test\",\n                },","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"230c7fa21b5f2fb326666b23d69493daf09e1006","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                ."},{"line_number":155,"context_line":"                ."},{"line_number":156,"context_line":"                ."},{"line_number":157,"context_line":"                \"locked\": {},"},{"line_number":158,"context_line":"                \"name\": \"surya-probes-001\","},{"line_number":159,"context_line":"                \"OS-EXT-SRV-ATTR:launch_index\": 0,"},{"line_number":160,"context_line":"                \"created\": \"2018-06-29T15:07:29Z\","}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_314cbf40","line":157,"range":{"start_line":157,"start_character":25,"end_line":157,"end_character":29},"updated":"2019-03-28 15:54:14.000000000","message":"missing fields in response based on any condition is something we should avoid.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"ffedf2e6d96d363a2d3fe9d40916628912d9068d","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                ."},{"line_number":155,"context_line":"                ."},{"line_number":156,"context_line":"                ."},{"line_number":157,"context_line":"                \"locked\": {},"},{"line_number":158,"context_line":"                \"name\": \"surya-probes-001\","},{"line_number":159,"context_line":"                \"OS-EXT-SRV-ATTR:launch_index\": 0,"},{"line_number":160,"context_line":"                \"created\": \"2018-06-29T15:07:29Z\","}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_ac9cd2bf","line":157,"range":{"start_line":157,"start_character":25,"end_line":157,"end_character":29},"in_reply_to":"5fc1f717_314cbf40","updated":"2019-03-28 16:19:18.000000000","message":"Please refer to https://review.openstack.org/#/c/638629/1/specs/train/approved/add-locked-reason.rst@110","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"0ec777e462dcd41c9e8e84f07a7062fa0c585a85","unresolved":false,"context_lines":[{"line_number":170,"context_line":""},{"line_number":171,"context_line":"Note that the \"locked_by\" will come from Instance.locked_by (\u0027owner\u0027 if owner"},{"line_number":172,"context_line":"and \u0027other\u0027 if admin (currently)) and \"locked_reason\" will come from"},{"line_number":173,"context_line":"Instance.system_meatada.locked_reason. These changes will be exposed from a new"},{"line_number":174,"context_line":"microversion. We will no longer use the Instance.locked boolean for the"},{"line_number":175,"context_line":"response from the new microversion."},{"line_number":176,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_df8eaf2c","line":173,"range":{"start_line":173,"start_character":16,"end_line":173,"end_character":23},"updated":"2019-03-28 07:03:49.000000000","message":"nit, s/meatada/metadata/","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"73366b42de145c5129f405149d39337328ae5173","unresolved":false,"context_lines":[{"line_number":170,"context_line":""},{"line_number":171,"context_line":"Note that the \"locked_by\" will come from Instance.locked_by (\u0027owner\u0027 if owner"},{"line_number":172,"context_line":"and \u0027other\u0027 if admin (currently)) and \"locked_reason\" will come from"},{"line_number":173,"context_line":"Instance.system_meatada.locked_reason. These changes will be exposed from a new"},{"line_number":174,"context_line":"microversion. We will no longer use the Instance.locked boolean for the"},{"line_number":175,"context_line":"response from the new microversion."},{"line_number":176,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_1adaea49","line":173,"range":{"start_line":173,"start_character":16,"end_line":173,"end_character":23},"in_reply_to":"5fc1f717_df8eaf2c","updated":"2019-03-29 10:07:30.000000000","message":"Done","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"0ec777e462dcd41c9e8e84f07a7062fa0c585a85","unresolved":false,"context_lines":[{"line_number":248,"context_line":"Work Items"},{"line_number":249,"context_line":"----------"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"#. Handle ``POST /servers/{server_id}/action`` on receiving an optional"},{"line_number":252,"context_line":"   \"reason\" parameter for lock action."},{"line_number":253,"context_line":"#. Expose the ``locked_by`` and ``locked_reason`` through"},{"line_number":254,"context_line":"   ``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":255,"context_line":"   ``POST /servers/{server_id}/action``  where the action is rebuild or lock"},{"line_number":256,"context_line":"   and ``PUT servers/{server_id}`` REST APIs after bumping the microversion."},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"Dependencies"},{"line_number":259,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_ff9b6b6f","line":256,"range":{"start_line":251,"start_character":3,"end_line":256,"end_character":76},"updated":"2019-03-28 07:03:49.000000000","message":"They will be in the same patch, due to the microversion.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"73366b42de145c5129f405149d39337328ae5173","unresolved":false,"context_lines":[{"line_number":248,"context_line":"Work Items"},{"line_number":249,"context_line":"----------"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"#. Handle ``POST /servers/{server_id}/action`` on receiving an optional"},{"line_number":252,"context_line":"   \"reason\" parameter for lock action."},{"line_number":253,"context_line":"#. Expose the ``locked_by`` and ``locked_reason`` through"},{"line_number":254,"context_line":"   ``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":255,"context_line":"   ``POST /servers/{server_id}/action``  where the action is rebuild or lock"},{"line_number":256,"context_line":"   and ``PUT servers/{server_id}`` REST APIs after bumping the microversion."},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"Dependencies"},{"line_number":259,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5fc1f717_3a280631","line":256,"range":{"start_line":251,"start_character":3,"end_line":256,"end_character":76},"in_reply_to":"5fc1f717_ff9b6b6f","updated":"2019-03-29 10:07:30.000000000","message":"sorry the first item was the client side patch and second item it for the server side patch.","commit_id":"a7e468a9328159df0f75bf4aee4e6283f5c91e7e"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"b1005b4d19ee0baf406869912ba04845b532ebaa","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"We plan to expose the ``locked_by`` and ``locked_reason`` information through"},{"line_number":94,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":95,"context_line":"``POST /servers/{server_id}/action``  where the action is rebuild or lock and"},{"line_number":96,"context_line":"``PUT servers/{server_id}`` REST APIs whose reponses will have a change in"},{"line_number":97,"context_line":"their \"locked\" key."},{"line_number":98,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_9304841a","line":95,"range":{"start_line":95,"start_character":0,"end_line":95,"end_character":73},"updated":"2019-03-29 14:16:21.000000000","message":"also should we really need to send the response for the lock API ? can\u0027t they just do show on the instance or something ?","commit_id":"dd8083e602fb5de7374e449df6ac147a31252c9f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"6346ecca2a5d7e6e3cd539d342058baf96345ea1","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"We plan to expose the ``locked_by`` and ``locked_reason`` information through"},{"line_number":94,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":95,"context_line":"``POST /servers/{server_id}/action``  where the action is rebuild or lock and"},{"line_number":96,"context_line":"``PUT servers/{server_id}`` REST APIs whose reponses will have a change in"},{"line_number":97,"context_line":"their \"locked\" key."},{"line_number":98,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_d6243a11","line":95,"range":{"start_line":95,"start_character":0,"end_line":95,"end_character":73},"in_reply_to":"5fc1f717_9304841a","updated":"2019-03-29 14:44:02.000000000","message":"oh lock response will be full server representation? I thought of just returning these two new fields but that also might not make much sense as the lock is not any resource and showing half server information is not a good way. showing full server representation in the lock response is something we should not do. \n\nI think we can drop the lock API response change completely.\n\nsorry for the confusion here. I did not notice in my previous comment that you added the lock API response.","commit_id":"dd8083e602fb5de7374e449df6ac147a31252c9f"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"c551cc209ecf944da3cd5c3800a65de27380bff2","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"We plan to expose the ``locked_by`` and ``locked_reason`` information through"},{"line_number":94,"context_line":"``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":95,"context_line":"``POST /servers/{server_id}/action``  where the action is rebuild or lock and"},{"line_number":96,"context_line":"``PUT servers/{server_id}`` REST APIs whose reponses will have a change in"},{"line_number":97,"context_line":"their \"locked\" key."},{"line_number":98,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_f659365b","line":95,"range":{"start_line":95,"start_character":0,"end_line":95,"end_character":73},"in_reply_to":"5fc1f717_d6243a11","updated":"2019-03-29 14:55:15.000000000","message":"@gmann: no problem, yea if there is a response it should be a full one like mriedem suggested just like we have for rebuild. So I guess I will drop this part.","commit_id":"dd8083e602fb5de7374e449df6ac147a31252c9f"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"5a14c22e0767faa4f477b719403226dc26e5988a","unresolved":false,"context_lines":[{"line_number":149,"context_line":"                ."},{"line_number":150,"context_line":"                ."},{"line_number":151,"context_line":"                ."},{"line_number":152,"context_line":"                \"locked\": {},"},{"line_number":153,"context_line":"                \"name\": \"surya-probes-001\","},{"line_number":154,"context_line":"                \"OS-EXT-SRV-ATTR:launch_index\": 0,"},{"line_number":155,"context_line":"                \"created\": \"2018-06-29T15:07:29Z\","}],"source_content_type":"text/x-rst","patch_set":7,"id":"5fc1f717_7abc4eba","line":152,"range":{"start_line":152,"start_character":16,"end_line":152,"end_character":29},"updated":"2019-03-29 10:15:09.000000000","message":"@dansmith, gmann: as for the format of the key changing:\n\n1) yea it was boolean before its becoming a dict.. I understand the backwards compatibility issue.\n2) but adding all the three keys separately is just making the information redundant when the instance is not locked (in my initial spec proposal I had all the three keys)\n\n {\n     locked: false,\n     locked_by: NULL,\n     locked_reason: \"\"\n }","commit_id":"dd8083e602fb5de7374e449df6ac147a31252c9f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f301bc41247e17129bb944544f5afb8f0fb01227","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"},{"line_number":80,"context_line":"get a new optional argument called ``reason`` which will accept the reason for"},{"line_number":81,"context_line":"locking the server and store this in the instances table in the database."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":84,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"5fc1f717_509cbc0d","line":81,"range":{"start_line":81,"start_character":41,"end_line":81,"end_character":50},"updated":"2019-04-01 11:14:34.000000000","message":"instances_system_metadata was mentioned above.","commit_id":"d87f7e7724e1103cba531b35a93c9f5264d0db50"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4d47c95f648ed20dfe0f622b0e0c36b9ef82526c","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"},{"line_number":80,"context_line":"get a new optional argument called ``reason`` which will accept the reason for"},{"line_number":81,"context_line":"locking the server and store this in the instances table in the database."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":84,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"5fc1f717_da868666","line":81,"range":{"start_line":81,"start_character":41,"end_line":81,"end_character":50},"in_reply_to":"5fc1f717_509cbc0d","updated":"2019-04-01 11:45:05.000000000","message":"Done","commit_id":"d87f7e7724e1103cba531b35a93c9f5264d0db50"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f301bc41247e17129bb944544f5afb8f0fb01227","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                ."},{"line_number":117,"context_line":"                \"locked\": {"},{"line_number":118,"context_line":"                    \"locked_by\": \u0027owner\u0027 or \u0027other\u0027,"},{"line_number":119,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":120,"context_line":"                },"},{"line_number":121,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":8,"id":"5fc1f717_e7bacbbe","line":118,"range":{"start_line":118,"start_character":33,"end_line":118,"end_character":52},"updated":"2019-04-01 11:14:34.000000000","message":"What will be the type of this field? A string with two possible value \u0027owner\u0027 or \u0027other\u0027?\n\nThe current API doc says: \"Policy defaults enable only users with the administrative role or the owner of the server to perform this operation. Cloud providers can change these permissions through the policy.json file.\" \nCould this means that with a policy.json change other than the owner and the admin can lock an instance? If yes, then what will be shown in this field?\n\nAlso the current code use \u0027admin\u0027 instead of \u0027other\u0027 https://github.com/openstack/nova/blob/95a87bce9fa7575c172a7d06344fd3cd070db587/nova/compute/api.py#L4029","commit_id":"d87f7e7724e1103cba531b35a93c9f5264d0db50"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"123f36c5214d248f75f96dfb4a90faeebcc16936","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                ."},{"line_number":117,"context_line":"                \"locked\": {"},{"line_number":118,"context_line":"                    \"locked_by\": \u0027owner\u0027 or \u0027other\u0027,"},{"line_number":119,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":120,"context_line":"                },"},{"line_number":121,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":8,"id":"5fc1f717_25bbe2c4","line":118,"range":{"start_line":118,"start_character":33,"end_line":118,"end_character":52},"in_reply_to":"5fc1f717_7ac992aa","updated":"2019-04-01 19:09:59.000000000","message":"yeah, what tssurya explained. \n\nmaybe it will be clear if you add the response changes in JSON schema representation also, something like(with current value type)- https://review.openstack.org/#/c/638629/3/specs/train/approved/add-locked-reason.rst@116","commit_id":"d87f7e7724e1103cba531b35a93c9f5264d0db50"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4d47c95f648ed20dfe0f622b0e0c36b9ef82526c","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                ."},{"line_number":116,"context_line":"                ."},{"line_number":117,"context_line":"                \"locked\": {"},{"line_number":118,"context_line":"                    \"locked_by\": \u0027owner\u0027 or \u0027other\u0027,"},{"line_number":119,"context_line":"                    \"locked_reason\": \"foo-test\","},{"line_number":120,"context_line":"                },"},{"line_number":121,"context_line":"                \"name\": \"surya-probes-001\","}],"source_content_type":"text/x-rst","patch_set":8,"id":"5fc1f717_7ac992aa","line":118,"range":{"start_line":118,"start_character":33,"end_line":118,"end_character":52},"in_reply_to":"5fc1f717_e7bacbbe","updated":"2019-04-01 11:45:05.000000000","message":"yeah in order to ensure we don\u0027t leak anything to the user if the lock was done by a non-owner ( today this is done either by admin or owner, but in future the admin role could have several scopes ) we will output \"other\" if it was not by the owner him/herself like admin or another user and \"owner\" if it was the owner.\n\nThe type would be string with two possible values \"owner\" and \"admin\" for this response field.\n\nsee https://review.openstack.org/#/c/638629/3/specs/train/approved/add-locked-reason.rst@117 for discussion on this decision.","commit_id":"d87f7e7724e1103cba531b35a93c9f5264d0db50"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f301bc41247e17129bb944544f5afb8f0fb01227","unresolved":false,"context_lines":[{"line_number":164,"context_line":"    }"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"Note that the \"locked_by\" will come from Instance.locked_by (\u0027owner\u0027 if owner"},{"line_number":167,"context_line":"and \u0027other\u0027 if admin (currently)) and \"locked_reason\" will come from"},{"line_number":168,"context_line":"Instance.system_metadata.locked_reason. These changes will be exposed from a"},{"line_number":169,"context_line":"new microversion. We will no longer use the Instance.locked boolean for the"},{"line_number":170,"context_line":"response from the new microversion."}],"source_content_type":"text/x-rst","patch_set":8,"id":"5fc1f717_274a13ac","line":167,"range":{"start_line":167,"start_character":5,"end_line":167,"end_character":10},"updated":"2019-04-01 11:14:34.000000000","message":"it is \u0027admin\u0027 currently\n https://github.com/openstack/nova/blob/95a87bce9fa7575c172a7d06344fd3cd070db587/nova/compute/api.py#L4029","commit_id":"d87f7e7724e1103cba531b35a93c9f5264d0db50"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4d47c95f648ed20dfe0f622b0e0c36b9ef82526c","unresolved":false,"context_lines":[{"line_number":164,"context_line":"    }"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"Note that the \"locked_by\" will come from Instance.locked_by (\u0027owner\u0027 if owner"},{"line_number":167,"context_line":"and \u0027other\u0027 if admin (currently)) and \"locked_reason\" will come from"},{"line_number":168,"context_line":"Instance.system_metadata.locked_reason. These changes will be exposed from a"},{"line_number":169,"context_line":"new microversion. We will no longer use the Instance.locked boolean for the"},{"line_number":170,"context_line":"response from the new microversion."}],"source_content_type":"text/x-rst","patch_set":8,"id":"5fc1f717_9a431e14","line":167,"range":{"start_line":167,"start_character":5,"end_line":167,"end_character":10},"in_reply_to":"5fc1f717_274a13ac","updated":"2019-04-01 11:45:05.000000000","message":"yeah locked_by column\u0027s value will either be admin or owner since its an enum hardcoded that way today. But when exposing this via the API, we will not reveal to the user if it was the admin or security_admin or system_admin who did the locking. The API will only show \"other\" in its response.","commit_id":"d87f7e7724e1103cba531b35a93c9f5264d0db50"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f301bc41247e17129bb944544f5afb8f0fb01227","unresolved":false,"context_lines":[{"line_number":166,"context_line":"Note that the \"locked_by\" will come from Instance.locked_by (\u0027owner\u0027 if owner"},{"line_number":167,"context_line":"and \u0027other\u0027 if admin (currently)) and \"locked_reason\" will come from"},{"line_number":168,"context_line":"Instance.system_metadata.locked_reason. These changes will be exposed from a"},{"line_number":169,"context_line":"new microversion. We will no longer use the Instance.locked boolean for the"},{"line_number":170,"context_line":"response from the new microversion."},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"Security impact"},{"line_number":173,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":8,"id":"5fc1f717_07e277a1","line":170,"range":{"start_line":169,"start_character":18,"end_line":170,"end_character":35},"updated":"2019-04-01 11:14:34.000000000","message":"but do we keep the value of that field up to date to serve older microversion?","commit_id":"d87f7e7724e1103cba531b35a93c9f5264d0db50"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4d47c95f648ed20dfe0f622b0e0c36b9ef82526c","unresolved":false,"context_lines":[{"line_number":166,"context_line":"Note that the \"locked_by\" will come from Instance.locked_by (\u0027owner\u0027 if owner"},{"line_number":167,"context_line":"and \u0027other\u0027 if admin (currently)) and \"locked_reason\" will come from"},{"line_number":168,"context_line":"Instance.system_metadata.locked_reason. These changes will be exposed from a"},{"line_number":169,"context_line":"new microversion. We will no longer use the Instance.locked boolean for the"},{"line_number":170,"context_line":"response from the new microversion."},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"Security impact"},{"line_number":173,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":8,"id":"5fc1f717_1a688e85","line":170,"range":{"start_line":169,"start_character":18,"end_line":170,"end_character":35},"in_reply_to":"5fc1f717_07e277a1","updated":"2019-04-01 11:45:05.000000000","message":"I guess we have to since we have a specific column in the database for that and also a bunch of actions which filter instances based on that value like deletion/rebuild etc..\n\nI can update this sentence to make this more clear.","commit_id":"d87f7e7724e1103cba531b35a93c9f5264d0db50"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f301bc41247e17129bb944544f5afb8f0fb01227","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        \"priority\": \"INFO\","},{"line_number":198,"context_line":"        \"publisher_id\": \"nova-api:fake-mini\""},{"line_number":199,"context_line":"    }"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"Other end user impact"},{"line_number":202,"context_line":"---------------------"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"5fc1f717_c7922ffd","line":200,"updated":"2019-04-01 11:14:34.000000000","message":"+1","commit_id":"d87f7e7724e1103cba531b35a93c9f5264d0db50"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":21,"context_line":"a description or mention a reason of why its being locked. This is often a much"},{"line_number":22,"context_line":"needed information in situations (eg. security/hardware team locking the server"},{"line_number":23,"context_line":"or when an automated job locks the server or an admin locks it and goes on"},{"line_number":24,"context_line":"vacation) when its not ideal to just ask the user why it was locked and if"},{"line_number":25,"context_line":"could be unlocked."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Use Cases"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_f1c42962","line":24,"range":{"start_line":24,"start_character":72,"end_line":24,"end_character":74},"updated":"2019-04-09 13:42:35.000000000","message":"if it","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"d10351991570f244f4e3d68fc2f9c84ed1a5151c","unresolved":false,"context_lines":[{"line_number":21,"context_line":"a description or mention a reason of why its being locked. This is often a much"},{"line_number":22,"context_line":"needed information in situations (eg. security/hardware team locking the server"},{"line_number":23,"context_line":"or when an automated job locks the server or an admin locks it and goes on"},{"line_number":24,"context_line":"vacation) when its not ideal to just ask the user why it was locked and if"},{"line_number":25,"context_line":"could be unlocked."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Use Cases"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_78c1f54d","line":24,"range":{"start_line":24,"start_character":72,"end_line":24,"end_character":74},"in_reply_to":"5fc1f717_f1c42962","updated":"2019-04-09 16:41:03.000000000","message":"Done","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"843e3b4b941332ef65a638ae862cff0a633e22be","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"None."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"REST API impact"},{"line_number":77,"context_line":"---------------"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_9bdb7beb","line":76,"updated":"2019-04-09 17:13:07.000000000","message":"Regarding filtering/sorting parameters, I see that \u0027locked\u0027 is ignored as a valid sort key, I\u0027m not sure why:\n\nhttps://github.com/openstack/nova/blob/d42a007425d9adb691134137e1e0b7dda356df62/nova/api/openstack/compute/schemas/servers.py#L530\n\nI\u0027m assuming because locked_by is the valid sort key (even though we don\u0027t expose it in the API today, so it\u0027s weird we have it as a sort key):\n\nhttps://github.com/openstack/nova/blob/d42a007425d9adb691134137e1e0b7dda356df62/nova/api/openstack/compute/schemas/servers.py#L541\n\nAs for filter parameters, locked_by is the only one we\u0027d honor today, presumably for the same reasons as above (but again it\u0027s weird):\n\nhttps://github.com/openstack/nova/blob/d42a007425d9adb691134137e1e0b7dda356df62/nova/api/openstack/compute/schemas/servers.py#L585\n\nFor the purposes of this change in this spec, I\u0027d think sorting/filtering on locked would be fine since it\u0027s just a boolean, but filtering/sorting on locked_by and locked_reason could be weird because locked_by could be \u0027admin or other\u0027 here but we don\u0027t store \u0027other\u0027 in the DB we store \u0027owner\u0027 so you\u0027d have to do some kind of negative filter, e.g.:\n\nselect * from instances where locked_by is not \u0027admin\u0027;\n\nAnd I think locked_reason is out of the question for filtering/sorting since it\u0027s not an indexable field in the DB, it\u0027s just in system_metadata.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d6bf67ad0fc4735e1f5ff29051075adfdd2e8ad3","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"None."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"REST API impact"},{"line_number":77,"context_line":"---------------"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_dec811a9","line":76,"in_reply_to":"5fc1f717_9bdb7beb","updated":"2019-04-09 17:30:00.000000000","message":"Yeah, I tried to find the reason of current behaviour(\u0027locked\u0027 not valid key and \u0027locked_by\u0027 is valid key for sort/filter) but could not find in original patch.\n\nI agree on your point. \u0027locked\u0027 is good to filter/sort for example to filtering all locked servers in the bottom and do not look into those. \n\n+1 on adding sort/filter for \u0027locked\u0027 and remove the \u0027locked_by\u0027 from the current list.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1113936af461a020e6bc8668d2a34a568912c6f0","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"None."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"REST API impact"},{"line_number":77,"context_line":"---------------"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_fe7dcd6c","line":76,"in_reply_to":"5fc1f717_9bdb7beb","updated":"2019-04-09 17:31:10.000000000","message":"okay, based on what we discussed on irc, I will add the filtering/sorting based on the boolean locked in this spec and we can forget about \"locked_by\" based filtering/sorting.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"042c6e5ac3bff1540163d464519941bfdad775d9","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"None."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"REST API impact"},{"line_number":77,"context_line":"---------------"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_fee94d7e","line":76,"in_reply_to":"5fc1f717_fe7dcd6c","updated":"2019-04-09 17:51:11.000000000","message":"Simple question but do we really need to expose the locked_by field in this spec at all? Is it really needed for the locked_reason change or is just a matter of \"while we\u0027re exposing a reason we might as well expose this other thing that\u0027s already in the DB too\"? Because locked_by seems to be adding unnecessary complexity to this, especially with the \"admin or other\" semantics and filtering/sorting questions. Also note that the API reference filter parameter description for locked_by today says it\u0027s either admin or owner and is admin-only:\n\n\"Filter the server list result by who locked the server, possible value could be admin or owner.\n\nThis parameter is only valid when specified by administrators. If non-admin users specify this parameter, it is ignored.\"\n\nAnyway, it\u0027s just a thought but this could be simpler if we ditch locked_by as part of this spec.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"526ccf9745b55a19e7d36ab674d783dcb2e3cb28","unresolved":false,"context_lines":[{"line_number":77,"context_line":"---------------"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"},{"line_number":80,"context_line":"get a new optional argument called ``reason`` which will accept the reason for"},{"line_number":81,"context_line":"locking the server and store this in the instance_system_metadata table in the"},{"line_number":82,"context_line":"database."},{"line_number":83,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_4e7318e8","line":80,"range":{"start_line":80,"start_character":37,"end_line":80,"end_character":43},"updated":"2019-04-09 13:18:08.000000000","message":"locked_reason","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":77,"context_line":"---------------"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"},{"line_number":80,"context_line":"get a new optional argument called ``reason`` which will accept the reason for"},{"line_number":81,"context_line":"locking the server and store this in the instance_system_metadata table in the"},{"line_number":82,"context_line":"database."},{"line_number":83,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_b1d22133","line":80,"range":{"start_line":80,"start_character":37,"end_line":80,"end_character":43},"in_reply_to":"5fc1f717_4e7318e8","updated":"2019-04-09 13:42:35.000000000","message":"L44 says \u0027reason\u0027 as well. Might be confusing the request vs response parameters.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"d10351991570f244f4e3d68fc2f9c84ed1a5151c","unresolved":false,"context_lines":[{"line_number":77,"context_line":"---------------"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"},{"line_number":80,"context_line":"get a new optional argument called ``reason`` which will accept the reason for"},{"line_number":81,"context_line":"locking the server and store this in the instance_system_metadata table in the"},{"line_number":82,"context_line":"database."},{"line_number":83,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_ddeb3cda","line":80,"range":{"start_line":80,"start_character":37,"end_line":80,"end_character":43},"in_reply_to":"5fc1f717_b1d22133","updated":"2019-04-09 16:41:03.000000000","message":"I am sorry for the confusion. All I meant here was the \"reason\" is like \"nova lock --reason\" and in the database/request/response it will be stored as \"locked_reason\" just like we have \"reason\" and \"disabled_reason\" for service-disable.\n\nBut its my bad that I mixed up the CLI parameter name with the REST API request.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"526ccf9745b55a19e7d36ab674d783dcb2e3cb28","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"},{"line_number":80,"context_line":"get a new optional argument called ``reason`` which will accept the reason for"},{"line_number":81,"context_line":"locking the server and store this in the instance_system_metadata table in the"},{"line_number":82,"context_line":"database."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"A sample JSON request for locking a server would look like this::"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_313b91d1","line":81,"updated":"2019-04-09 13:18:08.000000000","message":"I guess this has gone through several iterations, but IIUC this sentence should be more like:\n\n\"...will change since the value of the ``lock`` key can now be a dict containing key ``locked_reason`` whose value is a string describing the reason for locking the server. This reason will be stored in the instance_system_metadata table...\"\n\nOr you could just delete a bunch of this since you describe it a couple paragraphs down. Just keep the instance_system_metadata bit.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"The request for ``POST /servers/{server_id}/action`` will change since it will"},{"line_number":80,"context_line":"get a new optional argument called ``reason`` which will accept the reason for"},{"line_number":81,"context_line":"locking the server and store this in the instance_system_metadata table in the"},{"line_number":82,"context_line":"database."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"A sample JSON request for locking a server would look like this::"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_b1bb01e3","line":81,"in_reply_to":"5fc1f717_313b91d1","updated":"2019-04-09 13:42:35.000000000","message":"On the request today, you have a lock key and just None - the value is ignored:\n\nhttps://developer.openstack.org/api-ref/compute/?expanded\u003dlock-server-lock-action-detail#lock-server-lock-action\n\nThe request changes to an optional dict just like the migrate request in 2.56 which started taking an optional \u0027host\u0027 parameter.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":84,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    \"lock\": {"},{"line_number":87,"context_line":"       \"locked_reason\": \"because I am mad at belmiro\""},{"line_number":88,"context_line":"    }"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"The request body would either be a \"null\" object in case the reason is not"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_91b605f9","line":87,"range":{"start_line":87,"start_character":8,"end_line":87,"end_character":21},"updated":"2019-04-09 13:42:35.000000000","message":"So is it going to be \u0027locked_reason\u0027 or just \u0027reason\u0027 because both are used in the spec and it\u0027s a bit confusing.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"49fc7b80caa892640d4a26d440a2f0aab7d4f207","unresolved":false,"context_lines":[{"line_number":84,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    \"lock\": {"},{"line_number":87,"context_line":"       \"locked_reason\": \"because I am mad at belmiro\""},{"line_number":88,"context_line":"    }"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"The request body would either be a \"null\" object in case the reason is not"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_dec25fe9","line":87,"updated":"2019-04-09 09:05:44.000000000","message":"lol","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"d10351991570f244f4e3d68fc2f9c84ed1a5151c","unresolved":false,"context_lines":[{"line_number":84,"context_line":"A sample JSON request for locking a server would look like this::"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    \"lock\": {"},{"line_number":87,"context_line":"       \"locked_reason\": \"because I am mad at belmiro\""},{"line_number":88,"context_line":"    }"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"The request body would either be a \"null\" object in case the reason is not"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_d80a6977","line":87,"range":{"start_line":87,"start_character":8,"end_line":87,"end_character":21},"in_reply_to":"5fc1f717_91b605f9","updated":"2019-04-09 16:41:03.000000000","message":"locked_reason.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":102,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":103,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":104,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":105,"context_line":"would mean its locked."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"A sample JSON response would look like this for a locked server::"},{"line_number":108,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_3194f143","line":105,"range":{"start_line":105,"start_character":11,"end_line":105,"end_character":14},"updated":"2019-04-09 13:42:35.000000000","message":"it is","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"d10351991570f244f4e3d68fc2f9c84ed1a5151c","unresolved":false,"context_lines":[{"line_number":102,"context_line":"type of this key will be changed to a dict which will include the ``locked_by``"},{"line_number":103,"context_line":"and ``locked_reason`` information. So an empty dict would mean the instance"},{"line_number":104,"context_line":"is not locked and a dict with the ``locked_by`` and ``locked_reason`` info"},{"line_number":105,"context_line":"would mean its locked."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"A sample JSON response would look like this for a locked server::"},{"line_number":108,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_982861e2","line":105,"range":{"start_line":105,"start_character":11,"end_line":105,"end_character":14},"in_reply_to":"5fc1f717_3194f143","updated":"2019-04-09 16:41:03.000000000","message":"Done","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":134,"context_line":""},{"line_number":135,"context_line":"Note that if the instance is locked by a non-owner we do not expose this"},{"line_number":136,"context_line":"information to the owner/user and hence return \u0027other\u0027 as value for locked_by."},{"line_number":137,"context_line":"If the owner tries to unlock an admin locked instance it would result in a 403"},{"line_number":138,"context_line":"which is an indication that it was locked by user with an administrative role."},{"line_number":139,"context_line":"Similarly its the duty of the admin locking the instance to put information"},{"line_number":140,"context_line":"that can be user-visible in the reason because there is no protection there."},{"line_number":141,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_b189c126","line":138,"range":{"start_line":137,"start_character":0,"end_line":138,"end_character":78},"updated":"2019-04-09 13:42:35.000000000","message":"And this is how it\u0027s always worked - no change in behavior here.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":136,"context_line":"information to the owner/user and hence return \u0027other\u0027 as value for locked_by."},{"line_number":137,"context_line":"If the owner tries to unlock an admin locked instance it would result in a 403"},{"line_number":138,"context_line":"which is an indication that it was locked by user with an administrative role."},{"line_number":139,"context_line":"Similarly its the duty of the admin locking the instance to put information"},{"line_number":140,"context_line":"that can be user-visible in the reason because there is no protection there."},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"A sample JSON response would look like this for an unlocked server::"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_717fd916","line":139,"range":{"start_line":139,"start_character":10,"end_line":139,"end_character":13},"updated":"2019-04-09 13:42:35.000000000","message":"it is","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"d10351991570f244f4e3d68fc2f9c84ed1a5151c","unresolved":false,"context_lines":[{"line_number":136,"context_line":"information to the owner/user and hence return \u0027other\u0027 as value for locked_by."},{"line_number":137,"context_line":"If the owner tries to unlock an admin locked instance it would result in a 403"},{"line_number":138,"context_line":"which is an indication that it was locked by user with an administrative role."},{"line_number":139,"context_line":"Similarly its the duty of the admin locking the instance to put information"},{"line_number":140,"context_line":"that can be user-visible in the reason because there is no protection there."},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"A sample JSON response would look like this for an unlocked server::"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_581e79bb","line":139,"range":{"start_line":139,"start_character":10,"end_line":139,"end_character":13},"in_reply_to":"5fc1f717_717fd916","updated":"2019-04-09 16:41:03.000000000","message":"Done","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":137,"context_line":"If the owner tries to unlock an admin locked instance it would result in a 403"},{"line_number":138,"context_line":"which is an indication that it was locked by user with an administrative role."},{"line_number":139,"context_line":"Similarly its the duty of the admin locking the instance to put information"},{"line_number":140,"context_line":"that can be user-visible in the reason because there is no protection there."},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"A sample JSON response would look like this for an unlocked server::"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_51c89d53","line":140,"updated":"2019-04-09 13:42:35.000000000","message":"I\u0027m not sure what this last sentence is trying to say - protection from what? The non-admin seeing the locked reason?\n\nThat does raise a concern about admins putting something in the locked_reason meant for other admins, like \"Maintenance in host aggregate G in cell 5\" for example, but admins should probably be aware the locked_reason information is visible by the non-admin owner of the instance (unless we add a new policy rule to control that).","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"d10351991570f244f4e3d68fc2f9c84ed1a5151c","unresolved":false,"context_lines":[{"line_number":137,"context_line":"If the owner tries to unlock an admin locked instance it would result in a 403"},{"line_number":138,"context_line":"which is an indication that it was locked by user with an administrative role."},{"line_number":139,"context_line":"Similarly its the duty of the admin locking the instance to put information"},{"line_number":140,"context_line":"that can be user-visible in the reason because there is no protection there."},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"A sample JSON response would look like this for an unlocked server::"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_1838f19d","line":140,"in_reply_to":"5fc1f717_51c89d53","updated":"2019-04-09 16:41:03.000000000","message":"\u003e I\u0027m not sure what this last sentence is trying to say - protection\n \u003e from what? The non-admin seeing the locked reason?\n\nyes non-admin seeing the reason of locking of an admin.\n\n \u003e \n \u003e That does raise a concern about admins putting something in the\n \u003e locked_reason meant for other admins, like \"Maintenance in host\n \u003e aggregate G in cell 5\" for example, but admins should probably be\n \u003e aware the locked_reason information is visible by the non-admin\n \u003e owner of the instance (unless we add a new policy rule to control\n \u003e that).\n\nThis is exactly what I am saying.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":167,"context_line":"Note that the \"locked_by\" will come from Instance.locked_by (\u0027owner\u0027 if owner"},{"line_number":168,"context_line":"and \u0027other\u0027 if admin (currently)) and \"locked_reason\" will come from"},{"line_number":169,"context_line":"Instance.system_metadata.locked_reason. These changes will be exposed from a"},{"line_number":170,"context_line":"new microversion. We will no longer use the Instance.locked boolean for the"},{"line_number":171,"context_line":"response from the new microversion although we will continue to keep this field"},{"line_number":172,"context_line":"updated in the database since some of the server actions like deletion and"},{"line_number":173,"context_line":"rebuild use it as a filter to check if the user is allowed to perform the"},{"line_number":174,"context_line":"requested action on the instance."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"Security impact"},{"line_number":177,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_51cdfd5a","line":174,"range":{"start_line":170,"start_character":18,"end_line":174,"end_character":33},"updated":"2019-04-09 13:42:35.000000000","message":"Well not to mention I could lock the server with a reason using the new microversion and then the user using openstack CLI which defaults to 2.1 could be showing details about the server where the locked_reason/locked_by information isn\u0027t exposed.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":176,"context_line":"Security impact"},{"line_number":177,"context_line":"---------------"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"None."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"Notifications impact"},{"line_number":182,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_d10a4d88","line":179,"updated":"2019-04-09 13:42:35.000000000","message":"Well there is the edge case I mentioned above about admins putting details in locked_reason about the deployment but maybe that\u0027s just an obvious thing, i.e. \"don\u0027t expose details about your deployment in ways your non-admin users can see.\"","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"d10351991570f244f4e3d68fc2f9c84ed1a5151c","unresolved":false,"context_lines":[{"line_number":176,"context_line":"Security impact"},{"line_number":177,"context_line":"---------------"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"None."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"Notifications impact"},{"line_number":182,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_986dc133","line":179,"in_reply_to":"5fc1f717_d10a4d88","updated":"2019-04-09 16:41:03.000000000","message":"yea see above.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        \"payload\": {"},{"line_number":194,"context_line":"            \"$ref\": \"common_payloads/InstanceActionPayload.json#\","},{"line_number":195,"context_line":"            \"nova_object.data\": {"},{"line_number":196,"context_line":"                \"locked\": true,"},{"line_number":197,"context_line":"                \"locked_by\": admin,"},{"line_number":198,"context_line":"                \"locked_reason\": \"foo-test\""},{"line_number":199,"context_line":"            }"},{"line_number":200,"context_line":"        },"},{"line_number":201,"context_line":"        \"priority\": \"INFO\","}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_31271115","line":198,"range":{"start_line":196,"start_character":16,"end_line":198,"end_character":43},"updated":"2019-04-09 13:42:35.000000000","message":"Note that this is how dansmith wanted to see the new fields exposed in the server response body as well, so maybe that should have been noted in the REST API Impact section as an alternative to returning the locked dict format - I can see his argument there about avoid unnecessary change; maybe we should get feedback from some people on the API SIG and/or mordred (people that work a lot on SDKs and CLIs and such).","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"aea133b34b1232d41571d03c3ddaf31884c0ccc6","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        \"payload\": {"},{"line_number":194,"context_line":"            \"$ref\": \"common_payloads/InstanceActionPayload.json#\","},{"line_number":195,"context_line":"            \"nova_object.data\": {"},{"line_number":196,"context_line":"                \"locked\": true,"},{"line_number":197,"context_line":"                \"locked_by\": admin,"},{"line_number":198,"context_line":"                \"locked_reason\": \"foo-test\""},{"line_number":199,"context_line":"            }"},{"line_number":200,"context_line":"        },"},{"line_number":201,"context_line":"        \"priority\": \"INFO\","}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_c22fd9f3","line":198,"range":{"start_line":196,"start_character":16,"end_line":198,"end_character":43},"in_reply_to":"5fc1f717_31271115","updated":"2019-04-09 15:06:44.000000000","message":"I think this format might be easier on the REST side to consume - especially for statically typed languages like go. While we can certainly make it work in python sdk, there will need to be gymnastics to deal with clients getting a bool or a dict that would be avoided by just adding the locked_by and locked_reason fields.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0382e86e9eaba3d1d32d89b783f095ad29a77319","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        \"payload\": {"},{"line_number":194,"context_line":"            \"$ref\": \"common_payloads/InstanceActionPayload.json#\","},{"line_number":195,"context_line":"            \"nova_object.data\": {"},{"line_number":196,"context_line":"                \"locked\": true,"},{"line_number":197,"context_line":"                \"locked_by\": admin,"},{"line_number":198,"context_line":"                \"locked_reason\": \"foo-test\""},{"line_number":199,"context_line":"            }"},{"line_number":200,"context_line":"        },"},{"line_number":201,"context_line":"        \"priority\": \"INFO\","}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_fbb15fe1","line":198,"range":{"start_line":196,"start_character":16,"end_line":198,"end_character":43},"in_reply_to":"5fc1f717_986aa1b0","updated":"2019-04-09 17:05:14.000000000","message":"+1 on making it flat in API response too. I was thinking to remove \u0027locked\u0027 in favor of \u0027locked_by\u0027. if \u0027locked_by\u0027 is null then user can assume it is not locked. \n\nBut it will remove compatibility of old user so no strong opinion.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"d10351991570f244f4e3d68fc2f9c84ed1a5151c","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        \"payload\": {"},{"line_number":194,"context_line":"            \"$ref\": \"common_payloads/InstanceActionPayload.json#\","},{"line_number":195,"context_line":"            \"nova_object.data\": {"},{"line_number":196,"context_line":"                \"locked\": true,"},{"line_number":197,"context_line":"                \"locked_by\": admin,"},{"line_number":198,"context_line":"                \"locked_reason\": \"foo-test\""},{"line_number":199,"context_line":"            }"},{"line_number":200,"context_line":"        },"},{"line_number":201,"context_line":"        \"priority\": \"INFO\","}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_986aa1b0","line":198,"range":{"start_line":196,"start_character":16,"end_line":198,"end_character":43},"in_reply_to":"5fc1f717_c22fd9f3","updated":"2019-04-09 16:41:03.000000000","message":"OK, thanks for the review. I am going to update the spec based on this feedback.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9964fef4a957d2d21af28604a1dd15243d08300b","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        \"payload\": {"},{"line_number":194,"context_line":"            \"$ref\": \"common_payloads/InstanceActionPayload.json#\","},{"line_number":195,"context_line":"            \"nova_object.data\": {"},{"line_number":196,"context_line":"                \"locked\": true,"},{"line_number":197,"context_line":"                \"locked_by\": admin,"},{"line_number":198,"context_line":"                \"locked_reason\": \"foo-test\""},{"line_number":199,"context_line":"            }"},{"line_number":200,"context_line":"        },"},{"line_number":201,"context_line":"        \"priority\": \"INFO\","}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_9b285be2","line":198,"range":{"start_line":196,"start_character":16,"end_line":198,"end_character":43},"in_reply_to":"5fc1f717_fbb15fe1","updated":"2019-04-09 17:09:01.000000000","message":"\u003e +1 on making it flat in API response too. I was thinking to remove\n \u003e \u0027locked\u0027 in favor of \u0027locked_by\u0027. if \u0027locked_by\u0027 is null then user\n \u003e can assume it is not locked.\n \u003e \n \u003e But it will remove compatibility of old user so no strong opinion.\n\nThis was discussed in earlier patch sets and that\u0027s what led to changing the locked field from a boolean to a dict with the additional fields, but it sounds like we\u0027re moving away from that and just doing the easiest change necessary.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"* to add the new optional parameter ``--reason`` for"},{"line_number":212,"context_line":"  ``POST /servers/{server_id}/action`` where the action is \"lock\"."},{"line_number":213,"context_line":"* to accommodate the parsing of the new dict type \"locked\" key in the server"},{"line_number":214,"context_line":"  response for ``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":215,"context_line":"  ``POST /servers/{server_id}/action``  where the action is rebuild and"},{"line_number":216,"context_line":"  ``PUT servers/{server_id}`` REST APIs from the new microversion."}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_71ccb933","line":213,"updated":"2019-04-09 13:42:35.000000000","message":"Also to Dan\u0027s point about just exposing the two new fields (locked_by and locked_reason) we wouldn\u0027t have any changes for something like \"nova show\" if we didn\u0027t change the format of the \"locked\" parameter.\n\nI was thinking about this more with the change I made in 2.53 where os-services and os-hypervisors \u0027id\u0027 field changed from an int to a uuid and I can see how it\u0027s a bit confusing and we could have opted to just add a uuid field to the response on those APIs in 2.53, but it\u0027s slightly different because those needed the id to be uuid to uniquely identify the resources for things like DELETE /os-services/{service_id}. Anyway, I\u0027m just saying it\u0027s hard to know and we don\u0027t have hard rules about this.","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":228,"context_line":"Developer impact"},{"line_number":229,"context_line":"----------------"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"None."},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"Upgrade impact"},{"line_number":234,"context_line":"--------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_91b84575","line":231,"updated":"2019-04-09 13:42:35.000000000","message":"Well as noted SDK developers may have to handle the \u0027locked\u0027 field type change differently (bool to dict).","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fd95bf975296e4c5af986b0b119395325261b0d","unresolved":false,"context_lines":[{"line_number":248,"context_line":"----------"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"#. Handle ``POST /servers/{server_id}/action`` on receiving an optional"},{"line_number":251,"context_line":"   \"reason\" parameter for lock action on the client side."},{"line_number":252,"context_line":"#. Expose the ``locked_by`` and ``locked_reason`` through"},{"line_number":253,"context_line":"   ``GET servers/{server_id}``, ``GET /servers/detail``,"},{"line_number":254,"context_line":"   ``POST /servers/{server_id}/action``  where the action is rebuild"}],"source_content_type":"text/x-rst","patch_set":9,"id":"5fc1f717_51ae5d2e","line":251,"range":{"start_line":251,"start_character":4,"end_line":251,"end_character":10},"updated":"2019-04-09 13:42:35.000000000","message":"reason or locked_reason?","commit_id":"792c8a22a968f054f27c8fbfa9a57908838ccfc6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0946159a53bc6cbd217f76b0425588188bc11fe9","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"Filtering/Sorting: The \"locked\" key will be added to the existing list of valid"},{"line_number":176,"context_line":"sorting/filtering keys and exposed to the users so that instances can be"},{"line_number":177,"context_line":"filtered/sorted based on this field. Note that we will remove the \"locked_by\""},{"line_number":178,"context_line":"field from the existing `whitelist`_  since its anyways not exposed to users"},{"line_number":179,"context_line":"currently and since we map any \"non-owner\" value to \u0027other\u0027 in the API response"},{"line_number":180,"context_line":"we would have to do a negative filtering (all non-owner) for making it work."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"Security impact"},{"line_number":183,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":10,"id":"5fc1f717_fe3aade3","line":180,"range":{"start_line":177,"start_character":37,"end_line":180,"end_character":76},"updated":"2019-04-09 18:01:29.000000000","message":"Grumble, this doesn\u0027t seem right. As a user, I would definitely be interested in a report of all instances locked by me, or by not-me.\n\nBut I guess this has already been discussed to death?","commit_id":"54ee934b4f56324a06ef222134985e6b38ec8d1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6b78881fa5221b6e5626375e6bcae72ffca53ce8","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"Filtering/Sorting: The \"locked\" key will be added to the existing list of valid"},{"line_number":176,"context_line":"sorting/filtering keys and exposed to the users so that instances can be"},{"line_number":177,"context_line":"filtered/sorted based on this field. Note that we will remove the \"locked_by\""},{"line_number":178,"context_line":"field from the existing `whitelist`_  since its anyways not exposed to users"},{"line_number":179,"context_line":"currently and since we map any \"non-owner\" value to \u0027other\u0027 in the API response"},{"line_number":180,"context_line":"we would have to do a negative filtering (all non-owner) for making it work."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"Security impact"},{"line_number":183,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":10,"id":"5fc1f717_4a2c5798","line":180,"range":{"start_line":177,"start_character":37,"end_line":180,"end_character":76},"in_reply_to":"5fc1f717_fe3aade3","updated":"2019-04-09 20:12:24.000000000","message":"\u003e As a user, I would definitely be interested in a report of all instances locked by me, or by not-me.\n\nSure, but you can\u0027t get that from the current locked_by field value anyway, the value is either admin or owner where owner \u003d someone in the same project that owns the instance, not necessarily you the user (based on user_id):\n\nhttps://github.com/openstack/nova/blob/d42a007425d9adb691134137e1e0b7dda356df62/nova/compute/api.py#L4017\n\nAnyway, it\u0027s stuff like this that I think over complicates what locked_by is and how it should be used that it would be worth it\u0027s own spec if someone really cares about building out that part of the API.","commit_id":"54ee934b4f56324a06ef222134985e6b38ec8d1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"564ea097cde352032e7f8a6b937b0ed53b264059","unresolved":false,"context_lines":[{"line_number":18,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"When locking a server using the nova lock action there is no provision to set"},{"line_number":21,"context_line":"a description or mention a reason of why its being locked. This is often a much"},{"line_number":22,"context_line":"needed information in situations (eg. security/hardware team locking the server"},{"line_number":23,"context_line":"or when an automated job locks the server or an admin locks it and goes on"},{"line_number":24,"context_line":"vacation) when its not ideal to just ask the user why it was locked and if it"}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_eaa183f8","line":21,"range":{"start_line":21,"start_character":41,"end_line":21,"end_character":44},"updated":"2019-04-09 20:20:48.000000000","message":"nit: usually I\u0027m breaking all of the \"it\u0027s\" rules - here and elsewhere","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d74dbfb48afcfa67c9da3ee6abad0a4c4aa519a5","unresolved":false,"context_lines":[{"line_number":18,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"When locking a server using the nova lock action there is no provision to set"},{"line_number":21,"context_line":"a description or mention a reason of why its being locked. This is often a much"},{"line_number":22,"context_line":"needed information in situations (eg. security/hardware team locking the server"},{"line_number":23,"context_line":"or when an automated job locks the server or an admin locks it and goes on"},{"line_number":24,"context_line":"vacation) when its not ideal to just ask the user why it was locked and if it"}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_ca1da70f","line":21,"range":{"start_line":21,"start_character":41,"end_line":21,"end_character":44},"in_reply_to":"5fc1f717_eaa183f8","updated":"2019-04-09 20:28:39.000000000","message":"Done","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"564ea097cde352032e7f8a6b937b0ed53b264059","unresolved":false,"context_lines":[{"line_number":27,"context_line":"Use Cases"},{"line_number":28,"context_line":"---------"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"As a cloud operator I would like to know why a certain user with the admin role"},{"line_number":31,"context_line":"locked the server when that user is not contactable or without having to open"},{"line_number":32,"context_line":"internal tickets to find out."},{"line_number":33,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_8ab0ff41","line":30,"range":{"start_line":30,"start_character":59,"end_line":30,"end_character":79},"updated":"2019-04-09 20:20:48.000000000","message":"You don\u0027t have to have the admin role to lock servers in your own project even if another user in the same project locked the server, but I\u0027m splitting hairs here.","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d74dbfb48afcfa67c9da3ee6abad0a4c4aa519a5","unresolved":false,"context_lines":[{"line_number":27,"context_line":"Use Cases"},{"line_number":28,"context_line":"---------"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"As a cloud operator I would like to know why a certain user with the admin role"},{"line_number":31,"context_line":"locked the server when that user is not contactable or without having to open"},{"line_number":32,"context_line":"internal tickets to find out."},{"line_number":33,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_aa229b50","line":30,"range":{"start_line":30,"start_character":59,"end_line":30,"end_character":79},"in_reply_to":"5fc1f717_8ab0ff41","updated":"2019-04-09 20:28:39.000000000","message":"Done","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"68adbc1dff48ee2cd30b80b68d4823675ea47c95","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        ]"},{"line_number":161,"context_line":"    }"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"Filtering/Sorting: The \"locked\" key will be added to the existing list of valid"},{"line_number":164,"context_line":"sorting/filtering keys so that instances can be filtered/sorted based on this"},{"line_number":165,"context_line":"field."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"Security impact"},{"line_number":168,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_69c6056b","line":165,"range":{"start_line":163,"start_character":19,"end_line":165,"end_character":6},"updated":"2019-04-09 18:23:56.000000000","message":"also can we remove the \u0027locked_by\u0027 from valid keys as we are not exposing that?","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"53ad148e7d226009ca1834e2df5ee83e1427659a","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        ]"},{"line_number":161,"context_line":"    }"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"Filtering/Sorting: The \"locked\" key will be added to the existing list of valid"},{"line_number":164,"context_line":"sorting/filtering keys so that instances can be filtered/sorted based on this"},{"line_number":165,"context_line":"field."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"Security impact"},{"line_number":168,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_4950e932","line":165,"range":{"start_line":163,"start_character":19,"end_line":165,"end_character":6},"in_reply_to":"5fc1f717_29c8fd1f","updated":"2019-04-09 18:45:32.000000000","message":"\u003e for licked_by later.\n\nheh I do like how after all that discussion we ended up with this :)","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6b78881fa5221b6e5626375e6bcae72ffca53ce8","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        ]"},{"line_number":161,"context_line":"    }"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"Filtering/Sorting: The \"locked\" key will be added to the existing list of valid"},{"line_number":164,"context_line":"sorting/filtering keys so that instances can be filtered/sorted based on this"},{"line_number":165,"context_line":"field."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"Security impact"},{"line_number":168,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_6a47d34d","line":165,"range":{"start_line":163,"start_character":19,"end_line":165,"end_character":6},"in_reply_to":"5fc1f717_4950e932","updated":"2019-04-09 20:12:24.000000000","message":"\u003e licked_by\n\n/me calls human resources...","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5795fa8dde4a3b7615e1c023a4f3089f8af9dbaa","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        ]"},{"line_number":161,"context_line":"    }"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"Filtering/Sorting: The \"locked\" key will be added to the existing list of valid"},{"line_number":164,"context_line":"sorting/filtering keys so that instances can be filtered/sorted based on this"},{"line_number":165,"context_line":"field."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"Security impact"},{"line_number":168,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_29c8fd1f","line":165,"range":{"start_line":163,"start_character":19,"end_line":165,"end_character":6},"in_reply_to":"5fc1f717_69c6056b","updated":"2019-04-09 18:33:37.000000000","message":"as discussed on IRC, let\u0027s fix that when we finally plan something for licked_by later. ok for now.","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"564ea097cde352032e7f8a6b937b0ed53b264059","unresolved":false,"context_lines":[{"line_number":245,"context_line":"   action is rebuild and ``PUT servers/{server_id}`` REST APIs after"},{"line_number":246,"context_line":"   setting/deleting the reason while locking/unlocking and bumping the"},{"line_number":247,"context_line":"   microversion on the server side."},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"Dependencies"},{"line_number":250,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":251,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_4ad69751","line":248,"updated":"2019-04-09 20:20:48.000000000","message":"#. Support filtering/sorting on the \"locked\" parameter.","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d74dbfb48afcfa67c9da3ee6abad0a4c4aa519a5","unresolved":false,"context_lines":[{"line_number":245,"context_line":"   action is rebuild and ``PUT servers/{server_id}`` REST APIs after"},{"line_number":246,"context_line":"   setting/deleting the reason while locking/unlocking and bumping the"},{"line_number":247,"context_line":"   microversion on the server side."},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"Dependencies"},{"line_number":250,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":251,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_4ad77724","line":248,"in_reply_to":"5fc1f717_4ad69751","updated":"2019-04-09 20:28:39.000000000","message":"Done","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"564ea097cde352032e7f8a6b937b0ed53b264059","unresolved":false,"context_lines":[{"line_number":256,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"Unit and functional tests for verifying the functionality. Tempest schema test"},{"line_number":259,"context_line":"for changing the REST API request schema format."},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"Documentation Impact"},{"line_number":262,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_aac61b99","line":259,"range":{"start_line":259,"start_character":26,"end_line":259,"end_character":33},"updated":"2019-04-09 20:20:48.000000000","message":"response","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d74dbfb48afcfa67c9da3ee6abad0a4c4aa519a5","unresolved":false,"context_lines":[{"line_number":256,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"Unit and functional tests for verifying the functionality. Tempest schema test"},{"line_number":259,"context_line":"for changing the REST API request schema format."},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"Documentation Impact"},{"line_number":262,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":11,"id":"5fc1f717_ead98355","line":259,"range":{"start_line":259,"start_character":26,"end_line":259,"end_character":33},"in_reply_to":"5fc1f717_aac61b99","updated":"2019-04-09 20:28:39.000000000","message":"Done","commit_id":"1373c234f7ac1ff58fb2533213f114c4d7ffcd65"}]}