)]}'
{"nova/network/security_group/neutron_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fcd00a081bf90fc7a7c10e0d349fe6fb2cd2e844","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            security_group \u003d neutron.create_security_group("},{"line_number":54,"context_line":"                body).get(\u0027security_group\u0027)"},{"line_number":55,"context_line":"        except n_exc.BadRequest as e:"},{"line_number":56,"context_line":"            raise exception.Invalid(six.text_type(e))"},{"line_number":57,"context_line":"        except n_exc.NeutronClientException as e:"},{"line_number":58,"context_line":"            exc_info \u003d sys.exc_info()"}],"source_content_type":"text/x-python","patch_set":1,"id":"1af86dd1_698f1824","line":55,"updated":"2015-06-12 19:43:08.000000000","message":"This works, but probably the proper way to do this, given the compute API extension and the security group API, is to implement the validate_property method in the base class:\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/api/openstack/compute/contrib/security_groups.py#n199\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/network/security_group/security_group_base.py#n157\n\nSo you could add some logic in that method to enforce that name and description are provided (I\u0027m assuming \u0027name\u0027 is also required).\n\nSee how it\u0027s done for nova-network\u0027s security group API here:\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/compute/api.py#n3802","commit_id":"9b6b70bdf9cdad24d545888b7c775ca76f1da528"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"93f6d86148ee80771d1c66538a72a0842794c451","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            security_group \u003d neutron.create_security_group("},{"line_number":54,"context_line":"                body).get(\u0027security_group\u0027)"},{"line_number":55,"context_line":"        except n_exc.BadRequest as e:"},{"line_number":56,"context_line":"            raise exception.Invalid(six.text_type(e))"},{"line_number":57,"context_line":"        except n_exc.NeutronClientException as e:"},{"line_number":58,"context_line":"            exc_info \u003d sys.exc_info()"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32b979_0c2bf8ec","line":55,"in_reply_to":"1af86dd1_698f1824","updated":"2015-06-13 07:07:07.000000000","message":"@Matt\n\nYeah, security_group validation for Neutron is not implemented at all in Nova side now.\nHowever, I think the fact is not so bad because Nova and Neutron are different projects.\n\nThe validation for Neutron will be ported from Neutron code, but now the way is different from nova-network\u0027s one. For example, \"name\" is not required for Neutron\u0027s security_group API. \n\nFor example, we can create security_group without name like\n\n $ curl -g -i -X POST http://192.168.11.62:9696/v2.0/security-groups.json -d \u0027{\"security_group\": {}}\u0027 [..]\n\nand we can confirm the created security_group doesn\u0027t  contain the name:\n\n $ neutron  security-group-show c2316e97-c5e2-4751-ae0f-c145da2fc67d\n +----------------------+--------------------------------------------------------------------+\n | Field                | Value                                                              |\n +----------------------+--------------------------------------------------------------------+\n | description          |                                                                    |\n | id                   | c2316e97-c5e2-4751-ae0f-c145da2fc67d                                |\n | name                 |                                                                    |\n\n\nalex_xu and I tried to implement JSONSchema for security_group API in Kilo but we gave up it, because the way is different between nova-network and Neutron and it was difficult to implement the best validation for both.\nIn addition, if the validation is changed in Neutron side, maybe Nova needs to catch up the change if Nova contains Neutron\u0027s security_group validation. so I feel current situation that Neutron\u0027s validation doesn\u0027t exist in Nova is not so bad.\n\nAt least, Nova should consider Neutron returns BadRequest when passing a client request to Neutron. This patch just adds the error handling.","commit_id":"9b6b70bdf9cdad24d545888b7c775ca76f1da528"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4a610ef71b18fce921bdb43be9731a213714c87d","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            security_group \u003d neutron.create_security_group("},{"line_number":54,"context_line":"                body).get(\u0027security_group\u0027)"},{"line_number":55,"context_line":"        except n_exc.BadRequest as e:"},{"line_number":56,"context_line":"            raise exception.Invalid(six.text_type(e))"},{"line_number":57,"context_line":"        except n_exc.NeutronClientException as e:"},{"line_number":58,"context_line":"            exc_info \u003d sys.exc_info()"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32b979_2f2d67a7","line":55,"in_reply_to":"fa32b979_0c2bf8ec","updated":"2015-06-15 13:41:25.000000000","message":"\"In addition, if the validation is changed in Neutron side\"\n\nIf ^ happens it\u0027s a backwards incompatible change in Neutron which shouldn\u0027t happen.\n\nWe wouldn\u0027t have to check \u0027name\u0027 in validate_property, we could just check \u0027description\u0027.  I just thought that since the base class has this hook for doing the validation which the compute API extension calls (and the neutron security group API in nova isn\u0027t implementing), that would save us from getting here and calling to Neutron API just to get the 400.\n\nIt doesn\u0027t matter to me too much either way.  What do you think?","commit_id":"9b6b70bdf9cdad24d545888b7c775ca76f1da528"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"179ba259312ee246769ac4fac88952a85ccdaf2f","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            security_group \u003d neutron.create_security_group("},{"line_number":54,"context_line":"                body).get(\u0027security_group\u0027)"},{"line_number":55,"context_line":"        except n_exc.BadRequest as e:"},{"line_number":56,"context_line":"            raise exception.Invalid(six.text_type(e))"},{"line_number":57,"context_line":"        except n_exc.NeutronClientException as e:"},{"line_number":58,"context_line":"            exc_info \u003d sys.exc_info()"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32b979_528ad2c0","line":55,"in_reply_to":"fa32b979_2f2d67a7","updated":"2015-06-15 13:53:25.000000000","message":"Neutron team also is considering microversions based on Nova\u0027s one. So backwards incompatible changes will be able to happen.\n\nThe validation for Neutron\u0027s security_group is \"work in progress\" on https://review.openstack.org/#/c/189544/ .\nbut I don\u0027t feel the above patch will be always safe because microversions will be and it is difficult to catch all changes of Neutron security_groups due to difficult Neutron code.\n\nSo this patch makes Nova safe for avoiding a server internal error.","commit_id":"9b6b70bdf9cdad24d545888b7c775ca76f1da528"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"e15b994d7dd6c8ac42ba0a2aa4cbbbd67e087c9d","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            security_group \u003d neutron.create_security_group("},{"line_number":54,"context_line":"                body).get(\u0027security_group\u0027)"},{"line_number":55,"context_line":"        except n_exc.BadRequest as e:"},{"line_number":56,"context_line":"            raise exception.Invalid(six.text_type(e))"},{"line_number":57,"context_line":"        except n_exc.NeutronClientException as e:"},{"line_number":58,"context_line":"            exc_info \u003d sys.exc_info()"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32b979_067888f1","line":55,"in_reply_to":"fa32b979_4073d880","updated":"2015-06-15 14:56:12.000000000","message":"Yeah, nice point. I completely agree that Neutron/python-neutronclient works as minimum micro version as the default for avoiding unintentional incompatible changes.\nHowever, on nova-spec review of python-novaclient microversion s, current proposal changes default microversion to \"latest\" microversion. (I am against that) So it would be possible to do that on Neutron side.","commit_id":"9b6b70bdf9cdad24d545888b7c775ca76f1da528"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fa28e31be238e0114b1c9f771641f5f2be7e6880","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            security_group \u003d neutron.create_security_group("},{"line_number":54,"context_line":"                body).get(\u0027security_group\u0027)"},{"line_number":55,"context_line":"        except n_exc.BadRequest as e:"},{"line_number":56,"context_line":"            raise exception.Invalid(six.text_type(e))"},{"line_number":57,"context_line":"        except n_exc.NeutronClientException as e:"},{"line_number":58,"context_line":"            exc_info \u003d sys.exc_info()"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32b979_4073d880","line":55,"in_reply_to":"fa32b979_528ad2c0","updated":"2015-06-15 14:17:09.000000000","message":"Even if neutron adds microversions, nova shouldn\u0027t be automatically opted-in to using them via the client, you should have to specify the microversion you want with the default being 2.1 (or whatever) which is backwards compatible so you don\u0027t break clients, like nova.\n\nAnyway, I\u0027m fine with this smaller change.","commit_id":"9b6b70bdf9cdad24d545888b7c775ca76f1da528"}]}