)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"6385cb767a7b2d78aa2410cac46bf284a39d8716","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently lioadm helper creates portals with fixed IP addresses 0.0.0.0"},{"line_number":10,"context_line":"and ::0 and also uses fixed standard port 3260 ignoring any store"},{"line_number":11,"context_line":"configuration (iscsi_ip_address, iscsi_secondary_ip_addresses,"},{"line_number":12,"context_line":"iscsi_port) from the configuration as it should."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This could lead to problems if a different port was defined in the"},{"line_number":15,"context_line":"configuration as the target would be available only on 3260 and other"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7a016987_44166d56","line":12,"updated":"2015-05-27 09:52:06.000000000","message":"This sentence reads most naturally as saying that lioadm helper *should* ignore the store configuration, but that clearly is not what you mean.\n\nI suggest changing the wording as in the bug, to \"Currently lioadm helper creates ... instead of using store configuration (iscsi_ip_address, iscsi_secondary_ip_addresses,iscsi_port) from the configuration as it should.\"","commit_id":"245fbc4ea48e5b40dd5171d711d136f21111b3dd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"aaf880e6538c6bae800b7d5ac6a1dbf2aab12798","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently lioadm helper creates portals with fixed IP addresses 0.0.0.0"},{"line_number":10,"context_line":"and ::0 and also uses fixed standard port 3260 ignoring any store"},{"line_number":11,"context_line":"configuration (iscsi_ip_address, iscsi_secondary_ip_addresses,"},{"line_number":12,"context_line":"iscsi_port) from the configuration as it should."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This could lead to problems if a different port was defined in the"},{"line_number":15,"context_line":"configuration as the target would be available only on 3260 and other"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7a016987_627cb6b5","line":12,"in_reply_to":"7a016987_44166d56","updated":"2015-05-27 12:01:30.000000000","message":"Done","commit_id":"245fbc4ea48e5b40dd5171d711d136f21111b3dd"}],"cinder/cmd/rtstool.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6f008452c9a2f71bc8c092fd80f86479bf62a6e9","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            portal \u003d rtslib.NetworkPortal(tpg_new, ip, portals_port,"},{"line_number":83,"context_line":"                                          mode\u003d\u0027any\u0027)"},{"line_number":84,"context_line":"        except rtslib.utils.RTSLibError:"},{"line_number":85,"context_line":"            print(_(\u0027Error creating NetworkPortal: ensure port 3260 \u0027"},{"line_number":86,"context_line":"                    \u0027is not in use by another service.\u0027))"},{"line_number":87,"context_line":"            raise"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_fe07451b","line":85,"updated":"2015-03-05 11:29:45.000000000","message":"Thanks, I missed it.","commit_id":"c21f421a86ecf9edc248a813009eea5628b15722"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a2c681b6b0b46d3871e3f89dc85bcc997ef2ab25","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            portal \u003d rtslib.NetworkPortal(tpg_new, ip, portals_port,"},{"line_number":83,"context_line":"                                          mode\u003d\u0027any\u0027)"},{"line_number":84,"context_line":"        except rtslib.utils.RTSLibError:"},{"line_number":85,"context_line":"            print(_(\u0027Error creating NetworkPortal: ensure port 3260 \u0027"},{"line_number":86,"context_line":"                    \u0027is not in use by another service.\u0027))"},{"line_number":87,"context_line":"            raise"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_c53bade9","line":85,"updated":"2015-03-04 23:26:49.000000000","message":"This error message is no longer accurate given that the port can be changed.","commit_id":"c21f421a86ecf9edc248a813009eea5628b15722"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"94f6f417edc8b370bd1e2fab24ee99f68a8f0189","unresolved":false,"context_lines":[{"line_number":179,"context_line":"    print(sys.argv[0] + \" delete [iqn]\")"},{"line_number":180,"context_line":"    print(sys.argv[0] + \" verify\")"},{"line_number":181,"context_line":"    sys.exit(1)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"def parse_optional_create(argv):"},{"line_number":185,"context_line":"    optional_args \u003d {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a80dd14_3839a341","line":182,"updated":"2015-03-06 17:15:27.000000000","message":"I actually considered doing just what you say. :)\nBut then I discarded it because current implementation has positional arguments followed by an optional argument (initiator_iqn) with no short/long option prefix. So that would mean either creating a breaking change in rtstool CLI (for existing optional argument) or doing a mixed version with 2 types of optional arguments to keep backwards compatibility. Either way that should go in another patch since it\u0027s a refactor of this independent tool.","commit_id":"25dd0c2024b602200cae697ca89c4d55af2bcee4"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"34bcd03e4ea1a9c65cbcb4aceae43fba7e8b0e8e","unresolved":false,"context_lines":[{"line_number":179,"context_line":"    print(sys.argv[0] + \" delete [iqn]\")"},{"line_number":180,"context_line":"    print(sys.argv[0] + \" verify\")"},{"line_number":181,"context_line":"    sys.exit(1)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"def parse_optional_create(argv):"},{"line_number":185,"context_line":"    optional_args \u003d {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a80dd14_bded85b9","line":182,"updated":"2015-03-06 16:52:24.000000000","message":"I know that you didn\u0027t write the original code, but would you consider re-writing this using argparse? All of this parsing is ugly and prone to errors, the part in main where we check len(argv) as well, and the usage gets generated more cleanly.\nThis is really just a nit, so feel free to ignore me if you don\u0027t have the time or inclination to do this :)","commit_id":"25dd0c2024b602200cae697ca89c4d55af2bcee4"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"487edbf4d7f270329d91162e18b031b45253907c","unresolved":false,"context_lines":[{"line_number":179,"context_line":"    print(sys.argv[0] + \" delete [iqn]\")"},{"line_number":180,"context_line":"    print(sys.argv[0] + \" verify\")"},{"line_number":181,"context_line":"    sys.exit(1)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"def parse_optional_create(argv):"},{"line_number":185,"context_line":"    optional_args \u003d {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a80dd14_98e36fa3","line":182,"in_reply_to":"9a80dd14_3839a341","updated":"2015-03-06 17:21:04.000000000","message":"Ok, that makes sense. I\u0027ll work on the patch to use argparse and see how it goes. This sounds like it won\u0027t be possible to get in until \"L\" release anyway.","commit_id":"25dd0c2024b602200cae697ca89c4d55af2bcee4"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"f7a4744f7e1b5778946ac01e2c54a5f780748151","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def create(backing_device, name, userid, password, iser_enabled,"},{"line_number":37,"context_line":"           initiator_iqns\u003dNone, portals_ips\u003dNone, portals_port\u003d3260):"},{"line_number":38,"context_line":"    IPS_ALLOW_FAIL \u003d (\u0027::0\u0027,)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    try:"},{"line_number":41,"context_line":"        rtsroot \u003d rtslib.root.RTSRoot()"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_5f993ec3","line":38,"updated":"2015-05-20 05:39:42.000000000","message":"Would you comment what IPS_ALLOW_FAIL means?","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"71e0ec4dc2cded513bd0d81febb8373c8f4b5181","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def create(backing_device, name, userid, password, iser_enabled,"},{"line_number":37,"context_line":"           initiator_iqns\u003dNone, portals_ips\u003dNone, portals_port\u003d3260):"},{"line_number":38,"context_line":"    IPS_ALLOW_FAIL \u003d (\u0027::0\u0027,)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    try:"},{"line_number":41,"context_line":"        rtsroot \u003d rtslib.root.RTSRoot()"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_b1a85021","line":38,"in_reply_to":"7a016987_5f993ec3","updated":"2015-05-21 01:57:17.000000000","message":"I thought the name was self explanatory, but you are right, I should add original comment from line 91, otherwise I remove the TODO without actually doing it.","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"f7a4744f7e1b5778946ac01e2c54a5f780748151","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            portal \u003d rtslib.NetworkPortal(tpg_new, ip, portals_port,"},{"line_number":80,"context_line":"                                          mode\u003d\u0027any\u0027)"},{"line_number":81,"context_line":"        except rtslib.utils.RTSLibError:"},{"line_number":82,"context_line":"            if ip not in IPS_ALLOW_FAIL:"},{"line_number":83,"context_line":"                print(_(\u0027Error creating NetworkPortal: ensure port %(port)d \u0027"},{"line_number":84,"context_line":"                        \u0027on ip %(ip)s is not in use by another service.\u0027)"},{"line_number":85,"context_line":"                      % {\u0027port\u0027: portals_port, \u0027ip\u0027: ip})"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_9ff1f603","line":82,"updated":"2015-05-20 05:39:42.000000000","message":"I think even though the ip is in IPS_ALLOW_FAIL, it is better to print a warning. Since end user won\u0027t realize what ip is in IPS_ALLOW_FAIL if he/she doesn\u0027t read the code.","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"71e0ec4dc2cded513bd0d81febb8373c8f4b5181","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            portal \u003d rtslib.NetworkPortal(tpg_new, ip, portals_port,"},{"line_number":80,"context_line":"                                          mode\u003d\u0027any\u0027)"},{"line_number":81,"context_line":"        except rtslib.utils.RTSLibError:"},{"line_number":82,"context_line":"            if ip not in IPS_ALLOW_FAIL:"},{"line_number":83,"context_line":"                print(_(\u0027Error creating NetworkPortal: ensure port %(port)d \u0027"},{"line_number":84,"context_line":"                        \u0027on ip %(ip)s is not in use by another service.\u0027)"},{"line_number":85,"context_line":"                      % {\u0027port\u0027: portals_port, \u0027ip\u0027: ip})"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_d15d0c48","line":82,"in_reply_to":"7a016987_9ff1f603","updated":"2015-05-21 01:57:17.000000000","message":"I originally considered it but then decided to just keep original functionality which didn\u0027t log this because I assumed somebody decided it was not relevant. But since I\u0027m not alone in wanting it logged I\u0027ll gladly add it.","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"f7a4744f7e1b5778946ac01e2c54a5f780748151","unresolved":false,"context_lines":[{"line_number":84,"context_line":"                        \u0027on ip %(ip)s is not in use by another service.\u0027)"},{"line_number":85,"context_line":"                      % {\u0027port\u0027: portals_port, \u0027ip\u0027: ip})"},{"line_number":86,"context_line":"                raise"},{"line_number":87,"context_line":"        else:"},{"line_number":88,"context_line":"            try:"},{"line_number":89,"context_line":"                if iser_enabled \u003d\u003d \u0027True\u0027:"},{"line_number":90,"context_line":"                    portal.iser \u003d True"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_7ff63afa","line":87,"updated":"2015-05-20 05:39:42.000000000","message":"How about add a comment about the trick to use `else` here?","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"04f768fbee36311c8da80bb5dd4998fe50315b57","unresolved":false,"context_lines":[{"line_number":84,"context_line":"                        \u0027on ip %(ip)s is not in use by another service.\u0027)"},{"line_number":85,"context_line":"                      % {\u0027port\u0027: portals_port, \u0027ip\u0027: ip})"},{"line_number":86,"context_line":"                raise"},{"line_number":87,"context_line":"        else:"},{"line_number":88,"context_line":"            try:"},{"line_number":89,"context_line":"                if iser_enabled \u003d\u003d \u0027True\u0027:"},{"line_number":90,"context_line":"                    portal.iser \u003d True"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_c66bec83","line":87,"in_reply_to":"7a016987_31b46080","updated":"2015-05-22 11:57:52.000000000","message":"Got it. It\u0027s a useful language feature :-)","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"71e0ec4dc2cded513bd0d81febb8373c8f4b5181","unresolved":false,"context_lines":[{"line_number":84,"context_line":"                        \u0027on ip %(ip)s is not in use by another service.\u0027)"},{"line_number":85,"context_line":"                      % {\u0027port\u0027: portals_port, \u0027ip\u0027: ip})"},{"line_number":86,"context_line":"                raise"},{"line_number":87,"context_line":"        else:"},{"line_number":88,"context_line":"            try:"},{"line_number":89,"context_line":"                if iser_enabled \u003d\u003d \u0027True\u0027:"},{"line_number":90,"context_line":"                    portal.iser \u003d True"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_31b46080","line":87,"in_reply_to":"7a016987_7ff63afa","updated":"2015-05-21 01:57:17.000000000","message":"I don\u0027t understand what you mean by \"trick\", this is standard python else clause in try..except...else.","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"f7a4744f7e1b5778946ac01e2c54a5f780748151","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                           {\u0027file_path\u0027: destination_file})"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def parse_optional_create(argv):"},{"line_number":196,"context_line":"    optional_args \u003d {}"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    for arg in argv:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_3fdd626f","line":195,"updated":"2015-05-20 05:39:42.000000000","message":"Do you consider using argparse?","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"71e0ec4dc2cded513bd0d81febb8373c8f4b5181","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                           {\u0027file_path\u0027: destination_file})"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def parse_optional_create(argv):"},{"line_number":196,"context_line":"    optional_args \u003d {}"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    for arg in argv:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_f127e8c7","line":195,"in_reply_to":"7a016987_3fdd626f","updated":"2015-05-21 01:57:17.000000000","message":"Scott asked the same thing on patch 5. Here\u0027s my original reply: \"I actually considered doing just what you say. :) But then I discarded it because current implementation has positional arguments followed by an optional argument (initiator_iqn) with no short/long option prefix. So that would mean either creating a breaking change in rtstool CLI (for existing optional argument) or doing a mixed version with 2 types of optional arguments to keep backwards compatibility. Either way that should go in another patch since it\u0027s a refactor of this independent tool.\"","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6ae3ee7ea6f536509e08c9e137fd44291cf2e7e3","unresolved":false,"context_lines":[{"line_number":37,"context_line":"           initiator_iqns\u003dNone, portals_ips\u003dNone, portals_port\u003d3260):"},{"line_number":38,"context_line":"    # List of IPS that will not raise an error when they fail binding."},{"line_number":39,"context_line":"    # Originally we will fail on all binding errors."},{"line_number":40,"context_line":"    IPS_ALLOW_FAIL \u003d tuple()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    try:"},{"line_number":43,"context_line":"        rtsroot \u003d rtslib.root.RTSRoot()"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a016987_b17c4221","line":40,"updated":"2015-05-27 10:27:21.000000000","message":"Should be ips_allow_fail \u003d ()","commit_id":"245fbc4ea48e5b40dd5171d711d136f21111b3dd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"aaf880e6538c6bae800b7d5ac6a1dbf2aab12798","unresolved":false,"context_lines":[{"line_number":37,"context_line":"           initiator_iqns\u003dNone, portals_ips\u003dNone, portals_port\u003d3260):"},{"line_number":38,"context_line":"    # List of IPS that will not raise an error when they fail binding."},{"line_number":39,"context_line":"    # Originally we will fail on all binding errors."},{"line_number":40,"context_line":"    IPS_ALLOW_FAIL \u003d tuple()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    try:"},{"line_number":43,"context_line":"        rtsroot \u003d rtslib.root.RTSRoot()"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a016987_82c45240","line":40,"in_reply_to":"7a016987_b17c4221","updated":"2015-05-27 12:01:30.000000000","message":"Following PEP8: constants go in capital letters: https://www.python.org/dev/peps/pep-0008/#constants","commit_id":"245fbc4ea48e5b40dd5171d711d136f21111b3dd"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"0df2d08a948798576b82c02fcd9dacc0406e8990","unresolved":false,"context_lines":[{"line_number":37,"context_line":"           initiator_iqns\u003dNone, portals_ips\u003dNone, portals_port\u003d3260):"},{"line_number":38,"context_line":"    # List of IPS that will not raise an error when they fail binding."},{"line_number":39,"context_line":"    # Originally we will fail on all binding errors."},{"line_number":40,"context_line":"    IPS_ALLOW_FAIL \u003d tuple()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    try:"},{"line_number":43,"context_line":"        rtsroot \u003d rtslib.root.RTSRoot()"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_7dbe9b00","line":40,"updated":"2015-05-27 12:16:48.000000000","message":"I think Ivan is suggesting:\n\n   IPS_ALLOW_FAIL \u003d ()","commit_id":"5eca00ee4f15be5aeb353772db72f07cc7186b6b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"be746ef6d7ffd5e32f6aecc17c21dd6618639c73","unresolved":false,"context_lines":[{"line_number":37,"context_line":"           initiator_iqns\u003dNone, portals_ips\u003dNone, portals_port\u003d3260):"},{"line_number":38,"context_line":"    # List of IPS that will not raise an error when they fail binding."},{"line_number":39,"context_line":"    # Originally we will fail on all binding errors."},{"line_number":40,"context_line":"    IPS_ALLOW_FAIL \u003d tuple()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    try:"},{"line_number":43,"context_line":"        rtsroot \u003d rtslib.root.RTSRoot()"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_fdd04bcb","line":40,"in_reply_to":"7a016987_7dbe9b00","updated":"2015-05-27 12:31:08.000000000","message":"It\u0027s normal to get a -1 for not following guidelines,  that\u0027s why I thought he was referring to that, but it\u0027s extreme for using tuple() instead of (), which by the way I did because I think it\u0027s more readable.\n\nAnyway, that\u0027s a 0 vote, as agreed in Cinder meetings; unless someone is more interested in increasing his stats than actually helping.","commit_id":"5eca00ee4f15be5aeb353772db72f07cc7186b6b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87998d50a3870ed867a69288d512172928c224f2","unresolved":false,"context_lines":[{"line_number":37,"context_line":"           initiator_iqns\u003dNone, portals_ips\u003dNone, portals_port\u003d3260):"},{"line_number":38,"context_line":"    # List of IPS that will not raise an error when they fail binding."},{"line_number":39,"context_line":"    # Originally we will fail on all binding errors."},{"line_number":40,"context_line":"    IPS_ALLOW_FAIL \u003d tuple()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    try:"},{"line_number":43,"context_line":"        rtsroot \u003d rtslib.root.RTSRoot()"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_95900b97","line":40,"in_reply_to":"7a016987_c3b00c8e","updated":"2015-05-27 13:39:48.000000000","message":"That makes sense and you are right, if I change the value later on in the method I can no longer call it a constant.  XD\n\nI\u0027ll change it now. Thanks for pointing that out.","commit_id":"5eca00ee4f15be5aeb353772db72f07cc7186b6b"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"3cf4920b239dca321574b4f90322033dad1905da","unresolved":false,"context_lines":[{"line_number":37,"context_line":"           initiator_iqns\u003dNone, portals_ips\u003dNone, portals_port\u003d3260):"},{"line_number":38,"context_line":"    # List of IPS that will not raise an error when they fail binding."},{"line_number":39,"context_line":"    # Originally we will fail on all binding errors."},{"line_number":40,"context_line":"    IPS_ALLOW_FAIL \u003d tuple()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    try:"},{"line_number":43,"context_line":"        rtsroot \u003d rtslib.root.RTSRoot()"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_c3b00c8e","line":40,"in_reply_to":"7a016987_fdd04bcb","updated":"2015-05-27 13:15:29.000000000","message":"Gorka, thanks for addressing my comment. I won\u0027t -1 for () instead of tuple().\n\nI\u0027m confused a bit for using constants in such way: you define a variable, than assign new value for it in one place. It does\u0027n look like it\u0027s a constant. \n\nI\u0027ll +1/+2 for this patch if you\u0027ll move this line and move line #79 out of the method. \n\nAnyway, other cores could +1/+2 it as is.","commit_id":"5eca00ee4f15be5aeb353772db72f07cc7186b6b"}],"cinder/tests/targets/test_cxt_driver.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c09417d49d553921ecf1d5525b3e9861211f0264","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            with open(file_path, \u0027r\u0027) as cfg_file:"},{"line_number":192,"context_line":"                result \u003d cfg_file.read()"},{"line_number":193,"context_line":"                self.assertEqual(result, expected_file)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    @mock.patch(\u0027cinder.volume.targets.cxt.CxtAdm._get_target\u0027,"},{"line_number":196,"context_line":"                return_value\u003d1)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_b4af97e2","line":193,"updated":"2015-03-05 18:12:18.000000000","message":"If this is something we are enforcing now, say no more! :)\nNow that I look for it I\u0027ve found the bug about the order: #1259292\nThank you for pointing this out, or I would have kept increasing the amount of incorrect code. XD","commit_id":"13a284830444cb675a96e4f2494a21cffc908f33"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ae609c32e03ebe0e8fdf7a63ec96fb561b23c324","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            with open(file_path, \u0027r\u0027) as cfg_file:"},{"line_number":192,"context_line":"                result \u003d cfg_file.read()"},{"line_number":193,"context_line":"                self.assertEqual(result, expected_file)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    @mock.patch(\u0027cinder.volume.targets.cxt.CxtAdm._get_target\u0027,"},{"line_number":196,"context_line":"                return_value\u003d1)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_a345a318","line":193,"updated":"2015-03-05 17:23:06.000000000","message":"Nothing against switching order, but is there a specific reason to do it?\nI mean, in Python\u0027s unittest documentation examples they do it the way I have here: https://docs.python.org/2/library/unittest.html\nAnd in a lot of places in the currrent code you can see it also this way.\nI believe this is a personal preference in most cases unless we have specific guidelines in OpenStack for it. After all method\u0027s arguments are first and second: https://hg.python.org/cpython/file/cabd257b1070/Lib/unittest/case.py#l792","commit_id":"13a284830444cb675a96e4f2494a21cffc908f33"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"0d6ccf564dd4f9b1d87bf040a1072b9ab680a6a2","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            with open(file_path, \u0027r\u0027) as cfg_file:"},{"line_number":192,"context_line":"                result \u003d cfg_file.read()"},{"line_number":193,"context_line":"                self.assertEqual(result, expected_file)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    @mock.patch(\u0027cinder.volume.targets.cxt.CxtAdm._get_target\u0027,"},{"line_number":196,"context_line":"                return_value\u003d1)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_675900fe","line":193,"updated":"2015-03-05 15:00:54.000000000","message":"assertEqual order should be (EXPECTED, ACTUAL)","commit_id":"13a284830444cb675a96e4f2494a21cffc908f33"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"754dcfce93b389094c844db875c8f5a39c1f2027","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            with open(file_path, \u0027r\u0027) as cfg_file:"},{"line_number":192,"context_line":"                result \u003d cfg_file.read()"},{"line_number":193,"context_line":"                self.assertEqual(result, expected_file)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    @mock.patch(\u0027cinder.volume.targets.cxt.CxtAdm._get_target\u0027,"},{"line_number":196,"context_line":"                return_value\u003d1)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_4396a7f1","line":193,"in_reply_to":"9a80dd14_a345a318","updated":"2015-03-05 17:31:39.000000000","message":"Looking for the documentation now, but this is something we have been enforcing lately, and there have actually been patches to clean up unit tests specifically to correct this condition.\n\nWill post guidelines if/when I can find it.","commit_id":"13a284830444cb675a96e4f2494a21cffc908f33"},{"author":{"_account_id":12088,"name":"Anish Bhatt","email":"anish.7@gmail.com","username":"anish"},"change_message_id":"ccf01ea574ee824850d503d8d5c0ee62eb349865","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            with open(file_path, \u0027r\u0027) as cfg_file:"},{"line_number":192,"context_line":"                result \u003d cfg_file.read()"},{"line_number":193,"context_line":"                self.assertEqual(result, expected_file)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    @mock.patch(\u0027cinder.volume.targets.cxt.CxtAdm._get_target\u0027,"},{"line_number":196,"context_line":"                return_value\u003d1)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_a35e8374","line":193,"in_reply_to":"9a80dd14_a345a318","updated":"2015-03-05 17:30:41.000000000","message":"There is no official requirement as this is not enforcable via any hacking rules (or so I was told), but if you see assertEqual fail it always shows first argument as \u0027reference\u0027 and second as \u0027actual\u0027 which can be misleading if you pass them in the reverse order. Something like :\n\nMismatchError: !\u003d:\nreference \u003d {\u0027foo\u0027}\nactual    \u003d {\u0027bar\u0027}","commit_id":"13a284830444cb675a96e4f2494a21cffc908f33"}],"cinder/tests/targets/test_lio_driver.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ae609c32e03ebe0e8fdf7a63ec96fb561b23c324","unresolved":false,"context_lines":[{"line_number":313,"context_line":"            \u0027auth\u0027: \u0027CHAP %s %s\u0027 % (mock.sentinel.user, mock.sentinel.pwd),"},{"line_number":314,"context_line":"        }"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        self.assertEqual(result, expected_result)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        mock_create.assert_called_once_with("},{"line_number":319,"context_line":"            self.fake_iscsi_scan,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_c34b7729","line":316,"updated":"2015-03-05 17:23:06.000000000","message":"Line 301 also has it as actual, expected","commit_id":"13a284830444cb675a96e4f2494a21cffc908f33"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"0d6ccf564dd4f9b1d87bf040a1072b9ab680a6a2","unresolved":false,"context_lines":[{"line_number":313,"context_line":"            \u0027auth\u0027: \u0027CHAP %s %s\u0027 % (mock.sentinel.user, mock.sentinel.pwd),"},{"line_number":314,"context_line":"        }"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        self.assertEqual(result, expected_result)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        mock_create.assert_called_once_with("},{"line_number":319,"context_line":"            self.fake_iscsi_scan,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_47307c9e","line":316,"updated":"2015-03-05 15:00:54.000000000","message":"Switch to expected, actual.","commit_id":"13a284830444cb675a96e4f2494a21cffc908f33"},{"author":{"_account_id":12088,"name":"Anish Bhatt","email":"anish.7@gmail.com","username":"anish"},"change_message_id":"610b1e83e24f8e61eeaec8f7ca29d98d93f35524","unresolved":false,"context_lines":[{"line_number":313,"context_line":"            \u0027auth\u0027: \u0027CHAP %s %s\u0027 % (mock.sentinel.user, mock.sentinel.pwd),"},{"line_number":314,"context_line":"        }"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        self.assertEqual(result, expected_result)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        mock_create.assert_called_once_with("},{"line_number":319,"context_line":"            self.fake_iscsi_scan,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_0394afe0","line":316,"in_reply_to":"9a80dd14_c34b7729","updated":"2015-03-05 17:32:11.000000000","message":"Confusing when tests fail, please use (expected, actual) if possible.","commit_id":"13a284830444cb675a96e4f2494a21cffc908f33"}],"cinder/tests/unit/targets/targets_fixture.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6ae3ee7ea6f536509e08c9e137fd44291cf2e7e3","unresolved":false,"context_lines":[{"line_number":67,"context_line":"             \u0027size\u0027: 1,"},{"line_number":68,"context_line":"             \u0027id\u0027: self.fake_volume_id,"},{"line_number":69,"context_line":"             \u0027volume_type_id\u0027: None,"},{"line_number":70,"context_line":"             \u0027provider_location\u0027: (self.configuration.iscsi_ip_address + \u0027:\u0027 +"},{"line_number":71,"context_line":"                                   six.text_type(self.configuration.iscsi_port)"},{"line_number":72,"context_line":"                                   + \u0027iqn.2010-10.org.openstack:\u0027"},{"line_number":73,"context_line":"                                   \u0027volume-%s 2\u0027 % self.fake_volume_id),"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a016987_3104d27c","line":70,"updated":"2015-05-27 10:27:21.000000000","message":"String concatenation and formatting in a one line looks a bit strange. Please, use string formatting (%) only","commit_id":"245fbc4ea48e5b40dd5171d711d136f21111b3dd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"aaf880e6538c6bae800b7d5ac6a1dbf2aab12798","unresolved":false,"context_lines":[{"line_number":67,"context_line":"             \u0027size\u0027: 1,"},{"line_number":68,"context_line":"             \u0027id\u0027: self.fake_volume_id,"},{"line_number":69,"context_line":"             \u0027volume_type_id\u0027: None,"},{"line_number":70,"context_line":"             \u0027provider_location\u0027: (self.configuration.iscsi_ip_address + \u0027:\u0027 +"},{"line_number":71,"context_line":"                                   six.text_type(self.configuration.iscsi_port)"},{"line_number":72,"context_line":"                                   + \u0027iqn.2010-10.org.openstack:\u0027"},{"line_number":73,"context_line":"                                   \u0027volume-%s 2\u0027 % self.fake_volume_id),"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a016987_c2b00a8e","line":70,"in_reply_to":"7a016987_3104d27c","updated":"2015-05-27 12:01:30.000000000","message":"Done","commit_id":"245fbc4ea48e5b40dd5171d711d136f21111b3dd"}],"cinder/volume/targets/cxt.py":[{"author":{"_account_id":12088,"name":"Anish Bhatt","email":"anish.7@gmail.com","username":"anish"},"change_message_id":"6e298bad5e81e2152fc4d8b26db3e834fc764aaa","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        vol_id \u003d name.split(\u0027:\u0027)[1]"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        cfg_port \u003d kwargs.get(\u0027portals_port\u0027)"},{"line_number":162,"context_line":"        cfg_ips \u003d kwargs.get(\u0027portals_ips\u0027, [\u00270.0.0.0\u0027, \u0027::0\u0027])"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        portals \u003d \u0027,\u0027.join(map(lambda ip: self._get_portal(ip, cfg_port),"},{"line_number":165,"context_line":"                               cfg_ips))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_cf1b961a","line":162,"updated":"2015-03-05 18:39:35.000000000","message":"Close but no cigar, cxt won\u0027t work with 0.0.0.0 addresses, needs an actual IP always. self.configuration.iscsi_ip_address defaults to host_ip, hence that was used here.","commit_id":"6a7fa9d76f8ed0f7ef96d747553994bf34e0abc8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"278199ef0dbc74442b34c51fa22792c0ee39cd87","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        vol_id \u003d name.split(\u0027:\u0027)[1]"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        cfg_port \u003d kwargs.get(\u0027portals_port\u0027)"},{"line_number":162,"context_line":"        cfg_ips \u003d kwargs.get(\u0027portals_ips\u0027, [\u00270.0.0.0\u0027, \u0027::0\u0027])"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        portals \u003d \u0027,\u0027.join(map(lambda ip: self._get_portal(ip, cfg_port),"},{"line_number":165,"context_line":"                               cfg_ips))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_57755cf8","line":162,"updated":"2015-03-06 09:50:44.000000000","message":"Ok, I\u0027ll change this and resubmit.","commit_id":"6a7fa9d76f8ed0f7ef96d747553994bf34e0abc8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9183aadc256bb801f53fbc4113cd4a4c248d6bb7","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        vol_id \u003d name.split(\u0027:\u0027)[1]"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        cfg_port \u003d kwargs.get(\u0027portals_port\u0027)"},{"line_number":162,"context_line":"        cfg_ips \u003d kwargs.get(\u0027portals_ips\u0027, [\u00270.0.0.0\u0027, \u0027::0\u0027])"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        portals \u003d \u0027,\u0027.join(map(lambda ip: self._get_portal(ip, cfg_port),"},{"line_number":165,"context_line":"                               cfg_ips))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_400bf024","line":162,"updated":"2015-03-05 21:10:34.000000000","message":"We don\u0027t really need to set the default 0.0.0.0 since we\u0027ll always be getting \u0027portals_ips\u0027 from the driver, but I\u0027m always a bit paranoid about not setting a default. If that default is no good then I can just remove it.","commit_id":"6a7fa9d76f8ed0f7ef96d747553994bf34e0abc8"},{"author":{"_account_id":12088,"name":"Anish Bhatt","email":"anish.7@gmail.com","username":"anish"},"change_message_id":"f579fc7bab150dd2437a8d5dc868c0dcfcb2a1ee","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        vol_id \u003d name.split(\u0027:\u0027)[1]"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        cfg_port \u003d kwargs.get(\u0027portals_port\u0027)"},{"line_number":162,"context_line":"        cfg_ips \u003d kwargs.get(\u0027portals_ips\u0027, [\u00270.0.0.0\u0027, \u0027::0\u0027])"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        portals \u003d \u0027,\u0027.join(map(lambda ip: self._get_portal(ip, cfg_port),"},{"line_number":165,"context_line":"                               cfg_ips))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_97560bd1","line":162,"in_reply_to":"9a80dd14_400bf024","updated":"2015-03-05 21:58:02.000000000","message":"Yes, please remove it. 0.0.0.0 definitely will not work. I would rather not have a non-working default hardcoded in (as this might lead to some false assumptions)","commit_id":"6a7fa9d76f8ed0f7ef96d747553994bf34e0abc8"},{"author":{"_account_id":12088,"name":"Anish Bhatt","email":"anish.7@gmail.com","username":"anish"},"change_message_id":"6e298bad5e81e2152fc4d8b26db3e834fc764aaa","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        cfg_ips \u003d kwargs.get(\u0027portals_ips\u0027, [\u00270.0.0.0\u0027, \u0027::0\u0027])"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        portals \u003d \u0027,\u0027.join(map(lambda ip: self._get_portal(ip, cfg_port),"},{"line_number":165,"context_line":"                               cfg_ips))"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        if chap_auth is None:"},{"line_number":168,"context_line":"            volume_conf \u003d self.TARGET_FMT % (name, path, portals)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_0f168ef1","line":165,"updated":"2015-03-05 18:39:35.000000000","message":"Can this be more than one IP ? The correct format then would be \nPortalGroup\u003d1@\u003cfirst_ip\u003e\nPortalGroup\u003d2@\u003csecond_ip\u003e\n\nso on and so forth.","commit_id":"6a7fa9d76f8ed0f7ef96d747553994bf34e0abc8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"278199ef0dbc74442b34c51fa22792c0ee39cd87","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        cfg_ips \u003d kwargs.get(\u0027portals_ips\u0027, [\u00270.0.0.0\u0027, \u0027::0\u0027])"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        portals \u003d \u0027,\u0027.join(map(lambda ip: self._get_portal(ip, cfg_port),"},{"line_number":165,"context_line":"                               cfg_ips))"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        if chap_auth is None:"},{"line_number":168,"context_line":"            volume_conf \u003d self.TARGET_FMT % (name, path, portals)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_77726012","line":165,"updated":"2015-03-06 09:50:44.000000000","message":"I\u0027ve looked a little bit more and it seems that it\u0027s only some initiators that need to have multiple portal groups to support MPIO and sice that doesn\u0027t look to be the case in Nova I will leave this as it is PortalGroup\u003d1@\u003cfirst_ip:port\u003e,\u003csecond_ip:port\u003e","commit_id":"6a7fa9d76f8ed0f7ef96d747553994bf34e0abc8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9183aadc256bb801f53fbc4113cd4a4c248d6bb7","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        cfg_ips \u003d kwargs.get(\u0027portals_ips\u0027, [\u00270.0.0.0\u0027, \u0027::0\u0027])"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        portals \u003d \u0027,\u0027.join(map(lambda ip: self._get_portal(ip, cfg_port),"},{"line_number":165,"context_line":"                               cfg_ips))"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        if chap_auth is None:"},{"line_number":168,"context_line":"            volume_conf \u003d self.TARGET_FMT % (name, path, portals)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_600e3436","line":165,"updated":"2015-03-05 21:10:34.000000000","message":"Yes, it can be more than one IP and I actually read the documentation before coding this and it\u0027s a valid configuration setting PortalGroup\u003d1@ip1:port1,ip2:port2 as I\u0027m doing here.\nBut the more important thing is, is that what we really want to do? And your comment made me think if what we want is multiple ips on the same portal or multiple portal groups one for each IP.\nI think we should have multiple portals like you say because that way nova\u0027s multipath can work it\u0027s magic. Do you agree on this? I ask because if that\u0027s the case I have to change it not only here but also in lio\u0027s helper.","commit_id":"6a7fa9d76f8ed0f7ef96d747553994bf34e0abc8"},{"author":{"_account_id":12088,"name":"Anish Bhatt","email":"anish.7@gmail.com","username":"anish"},"change_message_id":"f579fc7bab150dd2437a8d5dc868c0dcfcb2a1ee","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        cfg_ips \u003d kwargs.get(\u0027portals_ips\u0027, [\u00270.0.0.0\u0027, \u0027::0\u0027])"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        portals \u003d \u0027,\u0027.join(map(lambda ip: self._get_portal(ip, cfg_port),"},{"line_number":165,"context_line":"                               cfg_ips))"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        if chap_auth is None:"},{"line_number":168,"context_line":"            volume_conf \u003d self.TARGET_FMT % (name, path, portals)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_b71cefc2","line":165,"in_reply_to":"9a80dd14_600e3436","updated":"2015-03-05 21:58:02.000000000","message":"I would think so as this is the configuration I\u0027ve mostly seen (multiple portal groups for multiple paths), but I\u0027ve never seen either approach being a hard requirement.\n\n Either configuration is valid for cxt, the chiscsi driver really won\u0027t care. I am not a 100% familiar with nova\u0027s multipathing (or multipathd in general) so I cannot comment on what is preferred here. I will leave this to your better judgement as either approach is a valid configuration.","commit_id":"6a7fa9d76f8ed0f7ef96d747553994bf34e0abc8"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"7ff0fdc93ac158bdda2e9b967cb974651f35a67c","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        if netutils.is_valid_ipv4(ip):"},{"line_number":142,"context_line":"            portal_ip \u003d ip"},{"line_number":143,"context_line":"        else:"},{"line_number":144,"context_line":"            portal_ip \u003d \u0027[\u0027 + ip + \u0027]\u0027"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        return portal_ip + portal_port"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9a80dd14_030c74a4","line":144,"updated":"2015-03-06 17:41:43.000000000","message":"I wonder if we need to be doing this somewhere on the LIO path as well.  (Not in this patch.)","commit_id":"25dd0c2024b602200cae697ca89c4d55af2bcee4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6c5173d128c04c26d8fce053c066b39afbf3ef12","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        if netutils.is_valid_ipv4(ip):"},{"line_number":142,"context_line":"            portal_ip \u003d ip"},{"line_number":143,"context_line":"        else:"},{"line_number":144,"context_line":"            portal_ip \u003d \u0027[\u0027 + ip + \u0027]\u0027"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        return portal_ip + portal_port"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9a80dd14_0b4d669a","line":144,"updated":"2015-03-06 19:40:31.000000000","message":"We don\u0027t really need to do it in LIO because we pass the port as a separate argument to rtstool CLI so it would be:\ncinder-rtstool create ...blahblah... -p3260 -aIPv4,IPv6","commit_id":"25dd0c2024b602200cae697ca89c4d55af2bcee4"},{"author":{"_account_id":12088,"name":"Anish Bhatt","email":"anish.7@gmail.com","username":"anish"},"change_message_id":"3517e5e50139de60940062acef39d49f340d6046","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        if netutils.is_valid_ipv4(ip):"},{"line_number":142,"context_line":"            portal_ip \u003d ip"},{"line_number":143,"context_line":"        else:"},{"line_number":144,"context_line":"            portal_ip \u003d \u0027[\u0027 + ip + \u0027]\u0027"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        return portal_ip + portal_port"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9a80dd14_ad01c56b","line":144,"in_reply_to":"9a80dd14_0b4d669a","updated":"2015-03-07 01:15:28.000000000","message":"Seems correct, cxt use ip:port, to differenciate between ipv6 adress two octet groups and port separator which both happen to be \u0027:\u0027, we wrap the ipv6 address in [ ]","commit_id":"25dd0c2024b602200cae697ca89c4d55af2bcee4"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"f7a4744f7e1b5778946ac01e2c54a5f780748151","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        if netutils.is_valid_ipv4(ip):"},{"line_number":123,"context_line":"            portal_ip \u003d ip"},{"line_number":124,"context_line":"        else:"},{"line_number":125,"context_line":"            portal_ip \u003d \u0027[\u0027 + ip + \u0027]\u0027"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        return portal_ip + portal_port"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_ffbfcaa7","line":125,"updated":"2015-05-20 05:39:42.000000000","message":"Could you add a comment why add square brackets, like the old code does?","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"04f768fbee36311c8da80bb5dd4998fe50315b57","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        if netutils.is_valid_ipv4(ip):"},{"line_number":123,"context_line":"            portal_ip \u003d ip"},{"line_number":124,"context_line":"        else:"},{"line_number":125,"context_line":"            portal_ip \u003d \u0027[\u0027 + ip + \u0027]\u0027"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        return portal_ip + portal_port"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_66d67850","line":125,"in_reply_to":"7a016987_719778ec","updated":"2015-05-22 11:57:52.000000000","message":"Oops. Thanks for pointing out!","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"71e0ec4dc2cded513bd0d81febb8373c8f4b5181","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        if netutils.is_valid_ipv4(ip):"},{"line_number":123,"context_line":"            portal_ip \u003d ip"},{"line_number":124,"context_line":"        else:"},{"line_number":125,"context_line":"            portal_ip \u003d \u0027[\u0027 + ip + \u0027]\u0027"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        return portal_ip + portal_port"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_719778ec","line":125,"in_reply_to":"7a016987_ffbfcaa7","updated":"2015-05-21 01:57:17.000000000","message":"The comment you mention is at line 118","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"f7a4744f7e1b5778946ac01e2c54a5f780748151","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        else:"},{"line_number":125,"context_line":"            portal_ip \u003d \u0027[\u0027 + ip + \u0027]\u0027"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        return portal_ip + portal_port"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def create_iscsi_target(self, name, tid, lun, path,"},{"line_number":130,"context_line":"                            chap_auth\u003dNone, **kwargs):"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_1fdaa617","line":127,"updated":"2015-05-20 05:39:42.000000000","message":"I think use \"%s:%s\" instead of \"+\" is better. Because here you appended \":\" to portal_port and \"[]\" to portal_ip, making them not a plain port number or ip address. When someone later tries to add more code here, it is easy to raise bug if he didn\u0027t not see line 120 and line 125.","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"71e0ec4dc2cded513bd0d81febb8373c8f4b5181","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        else:"},{"line_number":125,"context_line":"            portal_ip \u003d \u0027[\u0027 + ip + \u0027]\u0027"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        return portal_ip + portal_port"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def create_iscsi_target(self, name, tid, lun, path,"},{"line_number":130,"context_line":"                            chap_auth\u003dNone, **kwargs):"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_d1886cc7","line":127,"in_reply_to":"7a016987_1fdaa617","updated":"2015-05-21 01:57:17.000000000","message":"\"%s:%s\" wouldn\u0027t work because with default port (None) would result in \"ip:\" instead of \"ip\"","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"04f768fbee36311c8da80bb5dd4998fe50315b57","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        else:"},{"line_number":125,"context_line":"            portal_ip \u003d \u0027[\u0027 + ip + \u0027]\u0027"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        return portal_ip + portal_port"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def create_iscsi_target(self, name, tid, lun, path,"},{"line_number":130,"context_line":"                            chap_auth\u003dNone, **kwargs):"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_e6494832","line":127,"in_reply_to":"7a016987_d1886cc7","updated":"2015-05-22 11:57:52.000000000","message":"Got what you think.","commit_id":"c221d1861029f9f26ddb761654cee8e37b27c83d"}]}