)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"d8951a58c59d64e7274d59d8dff39318a6b97d49","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add possibility to remove chassis_uuid from a node"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Allow to unset the field \"chassis_uuid\" from a node using the \"ironic"},{"line_number":10,"context_line":"node-update \u003cnode_id\u003e remove chassis_uuid\" command."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I1c8406f83f9d240ede99b0458c5e8b6967f2e37a"},{"line_number":13,"context_line":"Closes-Bug: #1563263"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da6895a0_dc7838c7","line":10,"range":{"start_line":10,"start_character":13,"end_line":10,"end_character":20},"updated":"2016-10-25 15:03:53.000000000","message":"nit: node_uuid, not node_id (the term \"node_id\" is sometimes used as an internal attribute when referencing nodes within ironic itself; it\u0027s not something exposed to the user)","commit_id":"b7c4f6ed563e89a77f306b2e771a695384e42e06"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"2c9727aa8c8ee89f5afc9436054f9a7ba82ca47d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add possibility to remove chassis_uuid from a node"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Allow to unset the field \"chassis_uuid\" from a node using the \"ironic"},{"line_number":10,"context_line":"node-update \u003cnode_id\u003e remove chassis_uuid\" command."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I1c8406f83f9d240ede99b0458c5e8b6967f2e37a"},{"line_number":13,"context_line":"Closes-Bug: #1563263"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da6895a0_3005d5bb","line":10,"range":{"start_line":10,"start_character":13,"end_line":10,"end_character":20},"in_reply_to":"da6895a0_dc7838c7","updated":"2016-10-25 16:34:25.000000000","message":"Done","commit_id":"b7c4f6ed563e89a77f306b2e771a695384e42e06"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"7f4af851f5fc8f1fd79c778ca6b10c07687554a2","unresolved":false,"context_lines":[{"line_number":10,"context_line":"node-update \u003cnode_uuid\u003e remove chassis_uuid\" command."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I1c8406f83f9d240ede99b0458c5e8b6967f2e37a"},{"line_number":13,"context_line":"Closes-Bug: #1563263"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba5da102_dcbf0fa8","line":13,"updated":"2016-11-02 14:47:25.000000000","message":"since this change also requires a change to the client (CLI \u0026 OSC), it should be s/Closes/Partial/","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"39a3b717fe72d8ad9a7853fc99b07d348f7866a6","unresolved":false,"context_lines":[{"line_number":10,"context_line":"node-update \u003cnode_uuid\u003e remove chassis_uuid\" command."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I1c8406f83f9d240ede99b0458c5e8b6967f2e37a"},{"line_number":13,"context_line":"Closes-Bug: #1563263"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba5da102_a504fe17","line":13,"in_reply_to":"ba5da102_3c39ebf2","updated":"2016-11-02 15:43:03.000000000","message":"Oops. Dmitry explained this to me. This is good then. ignore my comment.","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"bb7b7d8332f8eae608248a7fc757034e4fe47644","unresolved":false,"context_lines":[{"line_number":10,"context_line":"node-update \u003cnode_uuid\u003e remove chassis_uuid\" command."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I1c8406f83f9d240ede99b0458c5e8b6967f2e37a"},{"line_number":13,"context_line":"Closes-Bug: #1563263"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba5da102_3c39ebf2","line":13,"in_reply_to":"ba5da102_dcbf0fa8","updated":"2016-11-02 14:53:12.000000000","message":"Ruby, this is not correct, this Closes/Partial is per-project in the bug, not per bug","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"}],"ironic/api/controllers/v1/node.py":[{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"d8951a58c59d64e7274d59d8dff39318a6b97d49","unresolved":false,"context_lines":[{"line_number":1174,"context_line":"            except AttributeError:"},{"line_number":1175,"context_line":"                if field \u003d\u003d \"chassis_id\":"},{"line_number":1176,"context_line":"                    rpc_node[field] \u003d None"},{"line_number":1177,"context_line":"                # Ignore fields that aren\u0027t exposed in the API"},{"line_number":1178,"context_line":"                continue"},{"line_number":1179,"context_line":"            if patch_val \u003d\u003d wtypes.Unset:"},{"line_number":1180,"context_line":"                patch_val \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"da6895a0_bf9d365b","line":1177,"updated":"2016-10-25 15:03:53.000000000","message":"nit: might want to update this comment to indicate what\u0027s happening above","commit_id":"b7c4f6ed563e89a77f306b2e771a695384e42e06"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"2c9727aa8c8ee89f5afc9436054f9a7ba82ca47d","unresolved":false,"context_lines":[{"line_number":1174,"context_line":"            except AttributeError:"},{"line_number":1175,"context_line":"                if field \u003d\u003d \"chassis_id\":"},{"line_number":1176,"context_line":"                    rpc_node[field] \u003d None"},{"line_number":1177,"context_line":"                # Ignore fields that aren\u0027t exposed in the API"},{"line_number":1178,"context_line":"                continue"},{"line_number":1179,"context_line":"            if patch_val \u003d\u003d wtypes.Unset:"},{"line_number":1180,"context_line":"                patch_val \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"da6895a0_500089aa","line":1177,"in_reply_to":"da6895a0_bf9d365b","updated":"2016-10-25 16:34:25.000000000","message":"Done","commit_id":"b7c4f6ed563e89a77f306b2e771a695384e42e06"},{"author":{"_account_id":11929,"name":"Michael Turek","email":"mjturek@linux.vnet.ibm.com","username":"mjturek"},"change_message_id":"bd8d7f30b8b77d80d7396ab3019a2a501abb59f1","unresolved":false,"context_lines":[{"line_number":1176,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1177,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1178,"context_line":"                if field \u003d\u003d \"chassis_id\":"},{"line_number":1179,"context_line":"                    rpc_node[field] \u003d None"},{"line_number":1180,"context_line":"                continue"},{"line_number":1181,"context_line":"            if patch_val \u003d\u003d wtypes.Unset:"},{"line_number":1182,"context_line":"                patch_val \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_653ca6c8","line":1179,"updated":"2016-11-02 15:37:34.000000000","message":"It seems like this isn\u0027t getting covered by the unit tests... Maybe add a test in unit.api.v1.test_nodes.TestPatch","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"37ba007f5fe97739ffc3d96e8fda6c789bc33a8a","unresolved":false,"context_lines":[{"line_number":1176,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1177,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1178,"context_line":"                if field \u003d\u003d \"chassis_id\":"},{"line_number":1179,"context_line":"                    rpc_node[field] \u003d None"},{"line_number":1180,"context_line":"                continue"},{"line_number":1181,"context_line":"            if patch_val \u003d\u003d wtypes.Unset:"},{"line_number":1182,"context_line":"                patch_val \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_ffe65e42","line":1179,"in_reply_to":"ba5da102_653ca6c8","updated":"2016-11-03 13:29:14.000000000","message":"I’ve added a unit test for this method. Let me know if you think more are needed.","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"1b1c2b1eb65b16ea92aac64dffd8d2447fb0e7f3","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if field \u003d\u003d \"chassis_id\":"},{"line_number":1194,"context_line":"                    rpc_node[field] \u003d None"},{"line_number":1195,"context_line":"                continue"},{"line_number":1196,"context_line":"            if patch_val \u003d\u003d wtypes.Unset:"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5da102_a7abbfd5","line":1193,"updated":"2016-11-04 17:28:39.000000000","message":"Shouldn\u0027t there be a guard method here, similar to https://github.com/openstack/ironic/commit/a7310377e27bbce454b8a7a6b943cc5b7a45bbad#diff-fcb18843d1a9ec5117a71319e3d89151R306 to enforce that you have to be the latest micro version to do this?","commit_id":"3dbe042063df55265aa307a0644b8c31d85fad03"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"23a1dc156166a996b159896fbad75a599301a38c","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if field \u003d\u003d \"chassis_id\":"},{"line_number":1194,"context_line":"                    rpc_node[field] \u003d None"},{"line_number":1195,"context_line":"                continue"},{"line_number":1196,"context_line":"            if patch_val \u003d\u003d wtypes.Unset:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a629dbe_397c4220","line":1193,"in_reply_to":"ba5da102_a7abbfd5","updated":"2016-11-07 12:49:28.000000000","message":"Done","commit_id":"3dbe042063df55265aa307a0644b8c31d85fad03"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b79fb78a095097c36048f5a8186ed05ceec30689","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if (api_utils.allow_remove_chassis_uuid() and"},{"line_number":1194,"context_line":"                        field \u003d\u003d \"chassis_id\"):"},{"line_number":1195,"context_line":"                    rpc_node[field] \u003d None"},{"line_number":1196,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_7277acd4","line":1193,"updated":"2016-11-07 15:24:09.000000000","message":"Should this throw some kind of error back to the client if you attempt to remove a chassis_uuid while using a micro version where the API did not permit that?","commit_id":"3762a32b71b2b94ecb780686ef2da1769fbfe0aa"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"23b1d9c1f2d6994aa14a0241e63ee93c27b3e254","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if (api_utils.allow_remove_chassis_uuid() and"},{"line_number":1194,"context_line":"                        field \u003d\u003d \"chassis_id\"):"},{"line_number":1195,"context_line":"                    rpc_node[field] \u003d None"},{"line_number":1196,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_f7a001a8","line":1193,"in_reply_to":"9a629dbe_7277acd4","updated":"2016-11-07 19:25:07.000000000","message":"++, we should return 406 I think.","commit_id":"3762a32b71b2b94ecb780686ef2da1769fbfe0aa"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"975e486f13117dcfa924e19ea68dd5aa6c97cb93","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if (api_utils.allow_remove_chassis_uuid() and"},{"line_number":1194,"context_line":"                        field \u003d\u003d \"chassis_id\"):"},{"line_number":1195,"context_line":"                    rpc_node[field] \u003d None"},{"line_number":1196,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_bd0e4c32","line":1193,"in_reply_to":"9a629dbe_f7a001a8","updated":"2016-11-08 11:46:15.000000000","message":"Done","commit_id":"3762a32b71b2b94ecb780686ef2da1769fbfe0aa"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"0dba0d1523c24f629d6ea2547958a23c5f9a2fc6","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if (field \u003d\u003d \"chassis_id\" and rpc_node[field] is not None):"},{"line_number":1194,"context_line":"                    if not api_utils.allow_remove_chassis_uuid():"},{"line_number":1195,"context_line":"                        raise exception.NotAcceptable()"},{"line_number":1196,"context_line":"                    rpc_node[field] \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_17646720","line":1193,"updated":"2016-11-08 16:33:24.000000000","message":"ooops, you do check that rpc_node contains chassis_id, but you don\u0027t check that node doesn\u0027t contain chassis_uuid, this should be added too. Having attribute error does not mean anything, as there is no chassis_id in the API node object. I don\u0027t really know why unittests pass...","commit_id":"fd86432ec50ee942adc2af44a3223f00114d6d68"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"bbaae9c5474fb3c4b5ae81649205c3110a8b7f91","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if (field \u003d\u003d \"chassis_id\" and rpc_node[field] is not None):"},{"line_number":1194,"context_line":"                    if not api_utils.allow_remove_chassis_uuid():"},{"line_number":1195,"context_line":"                        raise exception.NotAcceptable()"},{"line_number":1196,"context_line":"                    rpc_node[field] \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_ecfd3c3b","line":1193,"in_reply_to":"9a629dbe_17646720","updated":"2016-11-09 13:47:13.000000000","message":"From what I can tell, a chassis_id attribute is created on the fly in the Node init method (L #774). So when a new API node object is created, it has a chassis_id field, but the chassis_uuid field is unset (L #779) (or set to chassis_id if the field exists): that’s why the node here (that is created at L #1504) doesn’t contain chassis_uuid, but can contain a chassis_id. \nSo an AttributeError happens for chassis_id if no chassis is associated to the node, for instance when removing the chassis. \n\nThe chassis_id field is then replaced by chassis_uuid in the convert_with_links() method at the end of the patch() method (L #1524)\n\nPlease, let me know if I misunderstood something","commit_id":"fd86432ec50ee942adc2af44a3223f00114d6d68"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"09fb4abdae56718cd0b01078aad595ca290e4c26","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if (field \u003d\u003d \"chassis_id\" and rpc_node[field] is not None):"},{"line_number":1194,"context_line":"                    if not api_utils.allow_remove_chassis_uuid():"},{"line_number":1195,"context_line":"                        raise exception.NotAcceptable()"},{"line_number":1196,"context_line":"                    rpc_node[field] \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_d76458ba","line":1193,"in_reply_to":"9a629dbe_8ad01b21","updated":"2016-11-10 10:00:43.000000000","message":"No problem :)\nI’ve added a comment, but please, tell me if you think it’s not sufficient / clear enough.","commit_id":"fd86432ec50ee942adc2af44a3223f00114d6d68"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"1c73a0c3bb2d58ad0a5a42efaca2d3c3fea4af8c","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if (field \u003d\u003d \"chassis_id\" and rpc_node[field] is not None):"},{"line_number":1194,"context_line":"                    if not api_utils.allow_remove_chassis_uuid():"},{"line_number":1195,"context_line":"                        raise exception.NotAcceptable()"},{"line_number":1196,"context_line":"                    rpc_node[field] \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_8ad01b21","line":1193,"in_reply_to":"9a629dbe_a77b5a27","updated":"2016-11-09 16:01:11.000000000","message":"/o\\ You\u0027re right, sorry about that :) I\u0027d love to have a comment about that tho somewhere.","commit_id":"fd86432ec50ee942adc2af44a3223f00114d6d68"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"9ba6f37f7280cc067e6028da6ef39928ef32d81d","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if (field \u003d\u003d \"chassis_id\" and rpc_node[field] is not None):"},{"line_number":1194,"context_line":"                    if not api_utils.allow_remove_chassis_uuid():"},{"line_number":1195,"context_line":"                        raise exception.NotAcceptable()"},{"line_number":1196,"context_line":"                    rpc_node[field] \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_a77b5a27","line":1193,"in_reply_to":"9a629dbe_aa27a410","updated":"2016-11-09 15:31:11.000000000","message":"Yes, this code isn’t easy to understand :( \n\nI agree with you on what happens when removing chassis_uuid. \nSo, after the Node initialization is done, chassis_uuid is unset and chassis_id was not added. \nThat means that the AttributeError will be raised for chassis_id (https://github.com/openstack/ironic/blob/5cb06385f2f48e33bdf741e9bb17308949db5990/ironic/api/controllers/v1/node.py#L1175-L1176), and thus rpc_node[‘chassis_id’] will be set to None, which removes the chassis from the node.\n\nIf another operation is performed (let’s say adding a chassis), then chassis_uuid\u003d\u003cuuid\u003e is passed during the Node initialization, which means that:\n\n_chassis_uuid and chassis_id are set (https://github.com/openstack/ironic/blob/5cb06385f2f48e33bdf741e9bb17308949db5990/ironic/api/controllers/v1/node.py#L768 and https://github.com/openstack/ironic/blob/5cb06385f2f48e33bdf741e9bb17308949db5990/ironic/api/controllers/v1/node.py#L643-L647) \n\nand chassis_uuid is unset (or set to chassis_id from kwargs, which is unset) (https://github.com/openstack/ironic/blob/5cb06385f2f48e33bdf741e9bb17308949db5990/ironic/api/controllers/v1/node.py#L775). \n\nSo, no AttributeError is raised for chassis_id (so there is no change from previous behavior) and the chassis is added.\n\nDoes that seems correct?","commit_id":"fd86432ec50ee942adc2af44a3223f00114d6d68"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"0cb7ad8db348581767cf818a0504c7135285c533","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                # Ignore fields that aren\u0027t exposed in the API,"},{"line_number":1191,"context_line":"                # except chassis_id, which has to be removed from rpc_node"},{"line_number":1192,"context_line":"                # if it isn\u0027t set in node"},{"line_number":1193,"context_line":"                if (field \u003d\u003d \"chassis_id\" and rpc_node[field] is not None):"},{"line_number":1194,"context_line":"                    if not api_utils.allow_remove_chassis_uuid():"},{"line_number":1195,"context_line":"                        raise exception.NotAcceptable()"},{"line_number":1196,"context_line":"                    rpc_node[field] \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_aa27a410","line":1193,"in_reply_to":"9a629dbe_ecfd3c3b","updated":"2016-11-09 14:18:20.000000000","message":"This piece of code with chassis_uuid is really hard to follow :( My understanding is as follows:\n\nSo, in your case, when patch has removed chassis_uuid, chassis_uuid\u003dNone (or it was not in kwargs at all) was passed to Node constructor here https://github.com/openstack/ironic/blob/5cb06385f2f48e33bdf741e9bb17308949db5990/ironic/api/controllers/v1/node.py#L1485.\n\nThen, during Node initialization, chassis_id is not added to the Node object, as chassis_uuid None or Unset is passed - https://github.com/openstack/ironic/blob/5cb06385f2f48e33bdf741e9bb17308949db5990/ironic/api/controllers/v1/node.py#L639.\n\nThe fact that chassis_id was appended to fields does not mean that it is actually added to the Node, you need to call setattr(\u0027chassis_id\u0027, \u003csomething\u003e), and it was not called neither in https://github.com/openstack/ironic/blob/5cb06385f2f48e33bdf741e9bb17308949db5990/ironic/api/controllers/v1/node.py#L768, nor in https://github.com/openstack/ironic/blob/5cb06385f2f48e33bdf741e9bb17308949db5990/ironic/api/controllers/v1/node.py#L775.\n\nI may be wrong too, please share your findings if some of that ^^ is incorrect :)","commit_id":"fd86432ec50ee942adc2af44a3223f00114d6d68"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"7b5759c6c0f1bf38437ca5a7877d54d942213837","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"                # set (instead of chassis_uuid) if the node belongs to a"},{"line_number":1198,"context_line":"                # chassis: AttributeError is raised for chassis_id only if the"},{"line_number":1199,"context_line":"                # node doesn\u0027t belong to a chassis or if the chassis is being"},{"line_number":1200,"context_line":"                # removed."},{"line_number":1201,"context_line":"                if (field \u003d\u003d \"chassis_id\" and rpc_node[field] is not None):"},{"line_number":1202,"context_line":"                    if not api_utils.allow_remove_chassis_uuid():"},{"line_number":1203,"context_line":"                        raise exception.NotAcceptable()"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a629dbe_619817c1","line":1200,"updated":"2016-11-10 15:45:11.000000000","message":"I find the above confusing. Maybe\n\nIgnore fields that aren\u0027t exposed in the API, except chassis_id. chassis_id would have been set (instead of chassis_uuid) if the node belongs to a chassis. This AttributeError is raised for chassis_id only if 1. the node doesn\u0027t belong to a chassis or 2. the node belonged to a chassis but is now being removed from the chassis.\n\nThe above is how I interpret your comment. However, I don\u0027t understand case 1 -- if the node doesn\u0027t belong to a chassis, then chassis_id would NOT have been set. So something isn\u0027t quite right; I\u0027m thinking #1 is more like \u0027the node belonged to a chassis and is being changed to a different chassis?","commit_id":"43585098755f0d89b97bc751432ed4c241caf58e"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"2a98e715b79f70cf17dd3c6cf656571c5f4cd34e","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"                # set (instead of chassis_uuid) if the node belongs to a"},{"line_number":1198,"context_line":"                # chassis: AttributeError is raised for chassis_id only if the"},{"line_number":1199,"context_line":"                # node doesn\u0027t belong to a chassis or if the chassis is being"},{"line_number":1200,"context_line":"                # removed."},{"line_number":1201,"context_line":"                if (field \u003d\u003d \"chassis_id\" and rpc_node[field] is not None):"},{"line_number":1202,"context_line":"                    if not api_utils.allow_remove_chassis_uuid():"},{"line_number":1203,"context_line":"                        raise exception.NotAcceptable()"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a629dbe_370f3d89","line":1200,"in_reply_to":"9a629dbe_619817c1","updated":"2016-11-10 16:35:53.000000000","message":"Thanks for the suggestion Ruby, I’ve updated the comment. \n\nYou interpreted it correctly: in case 1 (the node doesn’t belong to a chassis and another unrelated operation is being executed), chassis_id is not set, just like you said, which means that the AttributeError will be raised.\nThat’s why we need to check that rpc_node[‘chassis_id’] is not already set to None (since that would mean that we are in case 2 and not in case 1): that way, if we are not actually removing the chassis, we just do ‘continue’ which was the previous behavior.\n\nAttributeError is not raised if 1. the chassis is being set, or 2. the chassis is being changed, or 3. the chassis is already set and another field is being updated.","commit_id":"43585098755f0d89b97bc751432ed4c241caf58e"}],"ironic/api/controllers/v1/versions.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"29e166f42d4e98bbbdb2a04580dc14936dd4a4bd","unresolved":false,"context_lines":[{"line_number":52,"context_line":"# v1.20: Add node.network_interface"},{"line_number":53,"context_line":"# v1.21: Add node.resource_class"},{"line_number":54,"context_line":"# v1.22: Ramdisk lookup and heartbeat endpoints."},{"line_number":55,"context_line":"# v1.23: Add possibility to unset chassis_uuid."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"MINOR_0_JUNO \u003d 0"},{"line_number":58,"context_line":"MINOR_1_INITIAL_VERSION \u003d 1"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_bc1edbb8","line":55,"updated":"2016-11-02 14:58:17.000000000","message":"s/./from node./\n\nor s/chassis_uuid/node.chassis_uuid/","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"37ba007f5fe97739ffc3d96e8fda6c789bc33a8a","unresolved":false,"context_lines":[{"line_number":52,"context_line":"# v1.20: Add node.network_interface"},{"line_number":53,"context_line":"# v1.21: Add node.resource_class"},{"line_number":54,"context_line":"# v1.22: Ramdisk lookup and heartbeat endpoints."},{"line_number":55,"context_line":"# v1.23: Add possibility to unset chassis_uuid."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"MINOR_0_JUNO \u003d 0"},{"line_number":58,"context_line":"MINOR_1_INITIAL_VERSION \u003d 1"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_ff33bef3","line":55,"in_reply_to":"ba5da102_bc1edbb8","updated":"2016-11-03 13:29:14.000000000","message":"Done","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"}],"ironic/tests/unit/api/v1/test_nodes.py":[{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"dd6bd409a32956388190f41c10bb4f80a7241133","unresolved":false,"context_lines":[{"line_number":1263,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node.uuid,"},{"line_number":1264,"context_line":"                                   [{\u0027path\u0027: \u0027/chassis_uuid\u0027,"},{"line_number":1265,"context_line":"                                     \u0027op\u0027: \u0027remove\u0027}])"},{"line_number":1266,"context_line":"        self.assertEqual(\u0027application/json\u0027, response.content_type)"},{"line_number":1267,"context_line":"        self.assertEqual(http_client.OK, response.status_code)"},{"line_number":1268,"context_line":""},{"line_number":1269,"context_line":"    def test_add_chassis_id(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_1259c7fe","line":1266,"updated":"2016-10-31 08:51:05.000000000","message":"I expected here to see that chassis_uuid is not present in new object, but other tests works in the same fashion.","commit_id":"3f94dc0dae3230df5a522197ba6f37ae550ee077"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"29e166f42d4e98bbbdb2a04580dc14936dd4a4bd","unresolved":false,"context_lines":[{"line_number":1264,"context_line":"                                   [{\u0027path\u0027: \u0027/chassis_uuid\u0027,"},{"line_number":1265,"context_line":"                                     \u0027op\u0027: \u0027remove\u0027}])"},{"line_number":1266,"context_line":"        self.assertEqual(\u0027application/json\u0027, response.content_type)"},{"line_number":1267,"context_line":"        self.assertEqual(http_client.OK, response.status_code)"},{"line_number":1268,"context_line":""},{"line_number":1269,"context_line":"    def test_add_chassis_id(self):"},{"line_number":1270,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_3f6c7dcf","line":1267,"updated":"2016-11-02 14:58:17.000000000","message":"seems somewhat incomplete, but maybe this is cuz some of the other tests seem incomplete too. Does the node already have chassis_uuid specified, and if so, wouldn\u0027t we want to check here that node.chassis_uuid is empty? (It has been awhile since I looked at these tests, maybe it isn\u0027t easy to do so?)","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"37ba007f5fe97739ffc3d96e8fda6c789bc33a8a","unresolved":false,"context_lines":[{"line_number":1264,"context_line":"                                   [{\u0027path\u0027: \u0027/chassis_uuid\u0027,"},{"line_number":1265,"context_line":"                                     \u0027op\u0027: \u0027remove\u0027}])"},{"line_number":1266,"context_line":"        self.assertEqual(\u0027application/json\u0027, response.content_type)"},{"line_number":1267,"context_line":"        self.assertEqual(http_client.OK, response.status_code)"},{"line_number":1268,"context_line":""},{"line_number":1269,"context_line":"    def test_add_chassis_id(self):"},{"line_number":1270,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_5f882a71","line":1267,"in_reply_to":"ba5da102_3f6c7dcf","updated":"2016-11-03 13:29:14.000000000","message":"From what I can tell, these tests mock the update_node() method (Line 1010) and so they don’t actually modify the node, which is why they only check the status_code. \nI’ve added a new test for _update_changed_fields() that checks if chassis_id is unset: let me know if you think more tests are needed.","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"bcd54b00232087ab402bc7a9f788d57b8a5e006f","unresolved":false,"context_lines":[{"line_number":1305,"context_line":"        node_no_chassis \u003d api_node.Node(**node_dict)"},{"line_number":1306,"context_line":""},{"line_number":1307,"context_line":"        controller._update_changed_fields(node_no_chassis, self.node)"},{"line_number":1308,"context_line":"        self.assertIsNone(self.node.chassis_id)"},{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"    def test_add_chassis_id(self):"},{"line_number":1311,"context_line":"        response \u003d self.patch_json(\u0027/nodes/%s\u0027 % self.node.uuid,"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_8101de6c","line":1308,"updated":"2016-11-09 17:48:51.000000000","message":"This is a good test; I don\u0027t see any other tests for controller._update_changed_fields.\n\nI think that if we added more tests like this, it could help with understanding the code (wrt the comments from vdrok). As a followup if anyone wants? And to test the raised exception.NotAcceptable.","commit_id":"fd86432ec50ee942adc2af44a3223f00114d6d68"}],"releasenotes/notes/add-chassis_uuid-removal-possibility-8b06341a91f7c676.yaml":[{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"d334b3e1f68af091fe7fe6c7230f1680aae1b599","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - Fixes an issue preventing the removal of the field \u0027chassis_uuid\u0027 from a"},{"line_number":4,"context_line":"    node. The field can now be unset using the \u0027ironic node-update \u003cnode_uuid\u003e"},{"line_number":5,"context_line":"    remove chassis_uuid\u0027 command."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"ba5da102_70d8341e","line":4,"updated":"2016-10-31 13:57:37.000000000","message":"Add possibility to unset chassis_uuid to OSC: https://review.openstack.org/#/c/391812","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"29e166f42d4e98bbbdb2a04580dc14936dd4a4bd","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - Fixes an issue preventing the removal of the field \u0027chassis_uuid\u0027 from a"},{"line_number":4,"context_line":"    node. The field can now be unset using the \u0027ironic node-update \u003cnode_uuid\u003e"},{"line_number":5,"context_line":"    remove chassis_uuid\u0027 command."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"ba5da102_dc2aaf8f","line":4,"in_reply_to":"ba5da102_70d8341e","updated":"2016-11-02 14:58:17.000000000","message":"This needs to change to be under \u0027features\u0027.\n\nPlease mention the microversion 1.23 where this is available.","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"37ba007f5fe97739ffc3d96e8fda6c789bc33a8a","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - Fixes an issue preventing the removal of the field \u0027chassis_uuid\u0027 from a"},{"line_number":4,"context_line":"    node. The field can now be unset using the \u0027ironic node-update \u003cnode_uuid\u003e"},{"line_number":5,"context_line":"    remove chassis_uuid\u0027 command."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"ba5da102_bf9946c6","line":4,"in_reply_to":"ba5da102_dc2aaf8f","updated":"2016-11-03 13:29:14.000000000","message":"Done","commit_id":"9fbaa31cb141a101bf62386c7782dc0fa29dfe24"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"23b1d9c1f2d6994aa14a0241e63ee93c27b3e254","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - Fixes an issue preventing the removal of the field \u0027chassis_uuid\u0027 from a"},{"line_number":4,"context_line":"    node. The field can now be unset using the \u0027ironic node-update \u003cnode_uuid\u003e"},{"line_number":5,"context_line":"    remove chassis_uuid\u0027 command. This is available with API version 1.25."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"9a629dbe_f7dc811a","line":2,"updated":"2016-11-07 19:25:07.000000000","message":"It is a fix :)","commit_id":"3762a32b71b2b94ecb780686ef2da1769fbfe0aa"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"975e486f13117dcfa924e19ea68dd5aa6c97cb93","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - Fixes an issue preventing the removal of the field \u0027chassis_uuid\u0027 from a"},{"line_number":4,"context_line":"    node. The field can now be unset using the \u0027ironic node-update \u003cnode_uuid\u003e"},{"line_number":5,"context_line":"    remove chassis_uuid\u0027 command. This is available with API version 1.25."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"9a629dbe_bd5c8c26","line":2,"in_reply_to":"9a629dbe_f7dc811a","updated":"2016-11-08 11:46:15.000000000","message":"I’m not sure about this: the launchpad bug (https://bugs.launchpad.net/ironic/+bug/1563263) has been changed to a RFE and it looks like there was a general agreement on this being a feature and not a bug (see rloo’s comment #3 in launchpad). \nWhat do you think?","commit_id":"3762a32b71b2b94ecb780686ef2da1769fbfe0aa"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"23b1d9c1f2d6994aa14a0241e63ee93c27b3e254","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - Fixes an issue preventing the removal of the field \u0027chassis_uuid\u0027 from a"},{"line_number":4,"context_line":"    node. The field can now be unset using the \u0027ironic node-update \u003cnode_uuid\u003e"},{"line_number":5,"context_line":"    remove chassis_uuid\u0027 command. This is available with API version 1.25."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"9a629dbe_d7c13d30","line":5,"updated":"2016-11-07 19:25:07.000000000","message":"starting with","commit_id":"3762a32b71b2b94ecb780686ef2da1769fbfe0aa"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"975e486f13117dcfa924e19ea68dd5aa6c97cb93","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - Fixes an issue preventing the removal of the field \u0027chassis_uuid\u0027 from a"},{"line_number":4,"context_line":"    node. The field can now be unset using the \u0027ironic node-update \u003cnode_uuid\u003e"},{"line_number":5,"context_line":"    remove chassis_uuid\u0027 command. This is available with API version 1.25."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"9a629dbe_fd10c450","line":5,"in_reply_to":"9a629dbe_d7c13d30","updated":"2016-11-08 11:46:15.000000000","message":"Done","commit_id":"3762a32b71b2b94ecb780686ef2da1769fbfe0aa"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"bcd54b00232087ab402bc7a9f788d57b8a5e006f","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - Fixes an issue preventing the removal of the field \u0027chassis_uuid\u0027 from a"},{"line_number":4,"context_line":"    node. The field can now be unset using the \u0027ironic node-update \u003cnode_uuid\u003e"},{"line_number":5,"context_line":"    remove chassis_uuid\u0027 command."},{"line_number":6,"context_line":"    This is available starting with API version 1.25."}],"source_content_type":"text/x-yaml","patch_set":7,"id":"9a629dbe_a18b42c2","line":6,"updated":"2016-11-09 17:48:51.000000000","message":"Hmm. This is a feature, not a bug. I\u0027m not sure I would use \u0027fixes\u0027 here. I also think if we mention the CLI, that we should be using OSC ones, not ironic ones.\n\nLooking at how existing release notes are written [1], how about something like:\n\n\"Adds support for removing the chassis UUID associated with a node (via PATCH /v1/nodes/\u003cident\u003e). This is available starting with API version 1.25.\"\n\nNot sure if we need to mention the CLIs.\n\n[1] http://docs.openstack.org/releasenotes/ironic/current-series.html","commit_id":"fd86432ec50ee942adc2af44a3223f00114d6d68"},{"author":{"_account_id":20522,"name":"Aline Bousquet","email":"albousque@gmail.com","username":"alineb"},"change_message_id":"09fb4abdae56718cd0b01078aad595ca290e4c26","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - Fixes an issue preventing the removal of the field \u0027chassis_uuid\u0027 from a"},{"line_number":4,"context_line":"    node. The field can now be unset using the \u0027ironic node-update \u003cnode_uuid\u003e"},{"line_number":5,"context_line":"    remove chassis_uuid\u0027 command."},{"line_number":6,"context_line":"    This is available starting with API version 1.25."}],"source_content_type":"text/x-yaml","patch_set":7,"id":"9a629dbe_7701ccae","line":6,"in_reply_to":"9a629dbe_a18b42c2","updated":"2016-11-10 10:00:43.000000000","message":"Done","commit_id":"fd86432ec50ee942adc2af44a3223f00114d6d68"}]}