)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"2c410e95f2ac52c5545cf7bc9099caa76db64162","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add DNS integration support to the client"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"DNS integration has been added to Neutron. This commit complements that by"},{"line_number":10,"context_line":"adding --dns-name and --dns-domain arguments to create and update commands of"},{"line_number":11,"context_line":"networks, ports and floating IPs."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bae84128_26575727","line":9,"updated":"2016-02-23 01:32:46.000000000","message":"Can you keep the limit the Message body to 72 characters?","commit_id":"0438a6341751117110caee4320b26f8e2c42989a"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"dad4fea6ccec5a92d64796185fc66725a385f08e","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add DNS integration support to the client"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"DNS integration has been added to Neutron. This commit complements that by"},{"line_number":10,"context_line":"adding --dns-name and --dns-domain arguments to create and update commands of"},{"line_number":11,"context_line":"networks, ports and floating IPs."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bae84128_a5b97317","line":9,"in_reply_to":"bae84128_26575727","updated":"2016-02-23 17:45:27.000000000","message":"Done","commit_id":"0438a6341751117110caee4320b26f8e2c42989a"}],"neutronclient/neutron/v2_0/dns.py":[{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"20cf47988e24c5b9b997da1efe10fea4b8682024","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"def add_dns_argument_create(parser, resource, attribute):"},{"line_number":20,"context_line":"    argument \u003d \u0027--dns-%s\u0027 % attribute"},{"line_number":21,"context_line":"    destination \u003d \u0027dns_%s\u0027 % attribute"},{"line_number":22,"context_line":"    parser.add_argument("},{"line_number":23,"context_line":"        argument, dest\u003ddestination,"},{"line_number":24,"context_line":"        help\u003d_(\u0027Assign DNS %(attribute)s to the %(resource)s \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"bae84128_7dcca535","line":21,"updated":"2016-02-20 06:09:26.000000000","message":"You do not need destination. That would be internally converted","commit_id":"8945dc48c2d4db390386b1c5056d814d0525a460"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"776a3e2d5884e631e78cd9eed819beae47538114","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"def add_dns_argument_create(parser, resource, attribute):"},{"line_number":20,"context_line":"    argument \u003d \u0027--dns-%s\u0027 % attribute"},{"line_number":21,"context_line":"    destination \u003d \u0027dns_%s\u0027 % attribute"},{"line_number":22,"context_line":"    parser.add_argument("},{"line_number":23,"context_line":"        argument, dest\u003ddestination,"},{"line_number":24,"context_line":"        help\u003d_(\u0027Assign DNS %(attribute)s to the %(resource)s \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"bae84128_cd77f138","line":21,"in_reply_to":"bae84128_7dcca535","updated":"2016-02-21 01:27:55.000000000","message":"Removed. I also removed it from the add_dns_argument_update method below","commit_id":"8945dc48c2d4db390386b1c5056d814d0525a460"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"20cf47988e24c5b9b997da1efe10fea4b8682024","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def args2body_dns_create(parsed_args, resource, attribute):"},{"line_number":30,"context_line":"    destination \u003d \u0027dns_%s\u0027 % attribute"},{"line_number":31,"context_line":"    argument \u003d getattr(parsed_args, destination, None)"},{"line_number":32,"context_line":"    if argument:"},{"line_number":33,"context_line":"        resource[destination] \u003d argument"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bae84128_5de601b5","line":31,"updated":"2016-02-20 06:09:26.000000000","message":"Should this be None, or argparse.SUPPRESS\nI think the latter, if you do not need to send this information to the neutron server when user doesnt pass it in the CLI","commit_id":"8945dc48c2d4db390386b1c5056d814d0525a460"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"776a3e2d5884e631e78cd9eed819beae47538114","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def args2body_dns_create(parsed_args, resource, attribute):"},{"line_number":30,"context_line":"    destination \u003d \u0027dns_%s\u0027 % attribute"},{"line_number":31,"context_line":"    argument \u003d getattr(parsed_args, destination, None)"},{"line_number":32,"context_line":"    if argument:"},{"line_number":33,"context_line":"        resource[destination] \u003d argument"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bae84128_6dc6a54a","line":31,"in_reply_to":"bae84128_5de601b5","updated":"2016-02-21 01:27:55.000000000","message":"I don\u0027t understand your comment. Are you saying it should be:\n\nargument \u003d getattr(parsed_args, destination, argparse.SUPPRESS)\n\nIf that is the case, how should I change line 32? And how should I change add_dns_argument_create?","commit_id":"8945dc48c2d4db390386b1c5056d814d0525a460"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"a341e3e53be8f2c070c115f86d418621fd505778","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def args2body_dns_create(parsed_args, resource, attribute):"},{"line_number":30,"context_line":"    destination \u003d \u0027dns_%s\u0027 % attribute"},{"line_number":31,"context_line":"    argument \u003d getattr(parsed_args, destination, None)"},{"line_number":32,"context_line":"    if argument:"},{"line_number":33,"context_line":"        resource[destination] \u003d argument"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bae84128_99087ddb","line":31,"in_reply_to":"bae84128_6dc6a54a","updated":"2016-02-21 23:39:00.000000000","message":"Reedip,\n\nI played with argparse.SUPPRESS this afternoon. If I modify my parser.add_call in line 22 and pass default\u003dargparse.SUPPRESS, then dns_name or dns_domain won\u0027t be an attribute of parsed_args in line 31. In that case, I do need the None argument in line 31, otherwise getattr will raise an AttributeError exception if the user didn\u0027t specify dns_name or dns_domain at the cli level.\n\nI chose to eliminate the None argument in line 31 and not pass default\u003dargparse.SUPPRESS in line 22. Please see PS3","commit_id":"8945dc48c2d4db390386b1c5056d814d0525a460"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"20cf47988e24c5b9b997da1efe10fea4b8682024","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        argument, dest\u003ddestination,"},{"line_number":43,"context_line":"        help\u003d_(\u0027Assign DNS %(attribute)s to the %(resource)s \u0027"},{"line_number":44,"context_line":"               \u0027(requires DNS integration \u0027"},{"line_number":45,"context_line":"               \u0027extension)\u0027) % {\u0027attribute\u0027: attribute, \u0027resource\u0027: resource})"},{"line_number":46,"context_line":"    dns_args.add_argument("},{"line_number":47,"context_line":"        no_argument, action\u003d\u0027store_true\u0027,"},{"line_number":48,"context_line":"        help\u003d_(\u0027Deassign DNS %(attribute)s from the %(resource)s \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"bae84128_bda46d57","line":45,"updated":"2016-02-20 06:09:26.000000000","message":"Period/Full stop at the end of the sentence","commit_id":"8945dc48c2d4db390386b1c5056d814d0525a460"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"776a3e2d5884e631e78cd9eed819beae47538114","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        argument, dest\u003ddestination,"},{"line_number":43,"context_line":"        help\u003d_(\u0027Assign DNS %(attribute)s to the %(resource)s \u0027"},{"line_number":44,"context_line":"               \u0027(requires DNS integration \u0027"},{"line_number":45,"context_line":"               \u0027extension)\u0027) % {\u0027attribute\u0027: attribute, \u0027resource\u0027: resource})"},{"line_number":46,"context_line":"    dns_args.add_argument("},{"line_number":47,"context_line":"        no_argument, action\u003d\u0027store_true\u0027,"},{"line_number":48,"context_line":"        help\u003d_(\u0027Deassign DNS %(attribute)s from the %(resource)s \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"bae84128_0d90b942","line":45,"in_reply_to":"bae84128_bda46d57","updated":"2016-02-21 01:27:55.000000000","message":"Done. I also added the period in line 50 below","commit_id":"8945dc48c2d4db390386b1c5056d814d0525a460"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"20cf47988e24c5b9b997da1efe10fea4b8682024","unresolved":false,"context_lines":[{"line_number":45,"context_line":"               \u0027extension)\u0027) % {\u0027attribute\u0027: attribute, \u0027resource\u0027: resource})"},{"line_number":46,"context_line":"    dns_args.add_argument("},{"line_number":47,"context_line":"        no_argument, action\u003d\u0027store_true\u0027,"},{"line_number":48,"context_line":"        help\u003d_(\u0027Deassign DNS %(attribute)s from the %(resource)s \u0027"},{"line_number":49,"context_line":"               \u0027(requires DNS integration \u0027"},{"line_number":50,"context_line":"               \u0027extension)\u0027) % {\u0027attribute\u0027: attribute, \u0027resource\u0027: resource})"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bae84128_fdad5532","line":48,"updated":"2016-02-20 06:09:26.000000000","message":"Unassign is better suited than deassign","commit_id":"8945dc48c2d4db390386b1c5056d814d0525a460"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"776a3e2d5884e631e78cd9eed819beae47538114","unresolved":false,"context_lines":[{"line_number":45,"context_line":"               \u0027extension)\u0027) % {\u0027attribute\u0027: attribute, \u0027resource\u0027: resource})"},{"line_number":46,"context_line":"    dns_args.add_argument("},{"line_number":47,"context_line":"        no_argument, action\u003d\u0027store_true\u0027,"},{"line_number":48,"context_line":"        help\u003d_(\u0027Deassign DNS %(attribute)s from the %(resource)s \u0027"},{"line_number":49,"context_line":"               \u0027(requires DNS integration \u0027"},{"line_number":50,"context_line":"               \u0027extension)\u0027) % {\u0027attribute\u0027: attribute, \u0027resource\u0027: resource})"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bae84128_ed92b54a","line":48,"in_reply_to":"bae84128_fdad5532","updated":"2016-02-21 01:27:55.000000000","message":"Done","commit_id":"8945dc48c2d4db390386b1c5056d814d0525a460"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"419ebfd1cfd73bbb7dc263167e74ae2d4aa9c53b","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    # Add dns_name and dns_domain support to network, port and floatingip"},{"line_number":32,"context_line":"    # create"},{"line_number":33,"context_line":"    destination \u003d \u0027dns_%s\u0027 % attribute"},{"line_number":34,"context_line":"    argument \u003d getattr(parsed_args, destination)"},{"line_number":35,"context_line":"    if argument:"},{"line_number":36,"context_line":"        resource[destination] \u003d argument"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_b904617e","line":34,"updated":"2016-02-22 00:16:54.000000000","message":"\u003c\u003cI move this comment to PS3 instead of PS1 so that other reviewers can see it easily\u003e\u003e\nHi Miguel,\nThe call is yours( and other reviewers) as to whether you want to pass dns_domain/dns_name in the body or not\nIMHO, if the user doesnt pass dns_domain or dns_name, you do not need to update it in the POST body to the server.\n\n\nTo do that, you can use hasattr instead of getattr in the Line #31 [1].\nLets see what other reviewers have to say about this.\n\n\n[1]: http://stackoverflow.com/questions/24971061/python-hasattr-vs-getattr","commit_id":"0438a6341751117110caee4320b26f8e2c42989a"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"db229c9ac1f9ef5a20d5fa47852af5ce2c7ecebe","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    # Add dns_name and dns_domain support to network, port and floatingip"},{"line_number":32,"context_line":"    # create"},{"line_number":33,"context_line":"    destination \u003d \u0027dns_%s\u0027 % attribute"},{"line_number":34,"context_line":"    argument \u003d getattr(parsed_args, destination)"},{"line_number":35,"context_line":"    if argument:"},{"line_number":36,"context_line":"        resource[destination] \u003d argument"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_d915b7e5","line":34,"in_reply_to":"bae84128_b904617e","updated":"2016-02-22 20:32:59.000000000","message":"Reedip,\n\nUnder this implementation, dns_domain or dns_name ARE NOT added to request body if the user didn\u0027t specify --dns-domain or --dns-name in the CLI. That is the point of lines 34 and 35. Let\u0027s use the example of creating a network without specifying --dns-domain. I entered --debug in the neutron net-create command and added a pdb.set_trace() statement right after line 34 (I remove some of the output lines for clarity):\n\n$ neutron net-create --debug my-net\n.....\n\u003e /opt/stack/python-neutronclient/neutronclient/neutron/v2_0/dns.py(37)args2body_dns_create()\n-\u003e if argument:\n(Pdb) parsed_args.dns_domain is None \nTrue\n(Pdb) argument is None\nTrue\n(Pdb) n\n--Return--\n\u003e /opt/stack/python-neutronclient/neutronclient/neutron/v2_0/dns.py(37)args2body_dns_create()-\u003eNone\n-\u003e if argument:\n\nAs you can see, line 36 was never executed and therefore, dns_name was never added to the request body. This is confirmed by the following lines:\n\n(Pdb) c\nDEBUG: keystoneauth.session REQ: curl -g -i -X POST http://9.114.111.101:9696/v2.0/networks.json -H \"User-Agent: python-neutronclient\" -H \"Content-Type: application/json\" -H \"Accept: application/json\" -H \"X-Auth-Token: {SHA1}196d24a5d5d449937bda2c187a36737dda93a1e0\" -d \u0027{\"network\": {\"name\": \"my-net\", \"admin_state_up\": true}}\u0027\nDEBUG: keystoneauth.session RESP: [201] Date: Mon, 22 Feb 2016 20:28:32 GMT Connection: keep-alive Content-Type: application/json; charset\u003dUTF-8 Content-Length: 321 X-Openstack-Request-Id: req-01821fb1-0bd2-4618-8e06-d3651e4f5f12 \nRESP BODY: {\"network\": {\"status\": \"ACTIVE\", \"subnets\": [], \"availability_zone_hints\": [], \"name\": \"my-net\", \"admin_state_up\": true, \"tenant_id\": \"d5660cb1e6934612a01b4fb2fb630725\", \"dns_domain\": \"\", \"mtu\": 1450, \"router:external\": false, \"shared\": false, \"port_security_enabled\": true, \"id\": \"23193958-cc1b-4a56-9e44-0e801dbf8e7b\"}}\n\nAs you can see above, the request body sent to the server is: {\"network\": {\"name\": \"my-net\", \"admin_state_up\": true}}","commit_id":"0438a6341751117110caee4320b26f8e2c42989a"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"74910346a17e95cef40cd88fa76e49cceddd688a","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    # Add dns_name and dns_domain support to network, port and floatingip"},{"line_number":32,"context_line":"    # create"},{"line_number":33,"context_line":"    destination \u003d \u0027dns_%s\u0027 % attribute"},{"line_number":34,"context_line":"    argument \u003d getattr(parsed_args, destination)"},{"line_number":35,"context_line":"    if argument:"},{"line_number":36,"context_line":"        resource[destination] \u003d argument"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_86474376","line":34,"in_reply_to":"bae84128_d915b7e5","updated":"2016-02-23 01:32:12.000000000","message":"@Miguel: Okay, I agree with the current behavior.\nas per my implementation, the parsed_args wont have the dns_domain or dns_name parameter, so you might have a larger chunk of code to handle  the condition.","commit_id":"0438a6341751117110caee4320b26f8e2c42989a"}]}