)]}'
{"cinder/tests/unit/test_hpe3par.py":[{"author":{"_account_id":11903,"name":"Anthony Lee","email":"anthony.mic.lee@hpe.com","username":"leeantho"},"change_message_id":"84d009e92ecd36a69ed73c8d9b46fcb3ffe7a4d6","unresolved":false,"context_lines":[{"line_number":6353,"context_line":"        # setup_mock_client drive with initiator CHAP disabled"},{"line_number":6354,"context_line":"        # and return the mock HTTP 3PAR client"},{"line_number":6355,"context_line":"        config \u003d self.setup_configuration()"},{"line_number":6356,"context_line":"        config.hpe3par_iscsi_chap_enabled \u003d True"},{"line_number":6357,"context_line":"        mock_client \u003d self.setup_driver(config\u003dconfig)"},{"line_number":6358,"context_line":"        mock_client.getVolume.return_value \u003d {\u0027userCPG\u0027: HPE3PAR_CPG}"},{"line_number":6359,"context_line":"        mock_client.getCPG.return_value \u003d {}"}],"source_content_type":"text/x-python","patch_set":7,"id":"7aa08908_0048a82c","line":6356,"range":{"start_line":6356,"start_character":8,"end_line":6356,"end_character":48},"updated":"2016-06-10 00:15:41.000000000","message":"The test is called \"test_create_host_chap_disabled\" but it looks like hpe_3par_iscsi_chap_enabled is being set to True here. It should be changed to False.","commit_id":"b23091700a3dc16241d62b3e11dc2520d5d233b6"}],"cinder/volume/drivers/hpe/hpe_3par_iscsi.py":[{"author":{"_account_id":6043,"name":"Kurt Martin","email":"kurt.f.martin@hpe.com","username":"kurt-f-martin"},"change_message_id":"4f03f8c102e527416dff7e65083ffc1df1106894","unresolved":false,"context_lines":[{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    \"\"\""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    VERSION \u003d \"3.0.10\""},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":123,"context_line":"        super(HPE3PARISCSIDriver, self).__init__(*args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab6814e_1d291738","line":120,"range":{"start_line":120,"start_character":15,"end_line":120,"end_character":22},"updated":"2016-05-24 23:19:55.000000000","message":"this version should be increased since you are changing the code and not just unit tests","commit_id":"a64abdbb9985fa197344a8ac311763df3a58af82"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"f40d7dc9e09e482b7e5d05ae39511c28e64f5afe","unresolved":false,"context_lines":[{"line_number":503,"context_line":"        used by 3PAR."},{"line_number":504,"context_line":"        \"\"\""},{"line_number":505,"context_line":"        # first search for an existing host"},{"line_number":506,"context_line":"        host_found \u003d None"},{"line_number":507,"context_line":"        if isinstance(iscsi_iqn, six.string_types):"},{"line_number":508,"context_line":"            iqn \u003d [iscsi_iqn]"},{"line_number":509,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7aa08908_b184615d","line":506,"range":{"start_line":506,"start_character":0,"end_line":506,"end_character":25},"updated":"2016-06-10 16:42:35.000000000","message":"I think the following could use some explanation maybe.  If I follow your 3Par lib expects a list of initiators.  There are two things that are kind of annoying here though:\n1. You\u0027re not holding a contract with the interface of your method params here.  You\u0027re saying \"give me a string, or a list... I don\u0027t care\"  that\u0027s not a good thing IMO.  Even with a private method I think you should be clear about what you expect and what the parameter type is.  Even though Python let\u0027s you do whatever you want, doesn\u0027t mean you should.\n\n2. The connector object currently is a single string, I don\u0027t believe the parameter currently can be a list anyway, so that means although you have an if/else here, your call is always going to be a string and always need the conversion no?\n\nWhy not enforce the param to be what you expect/need here?  In other words, in _create_host do \"[connector[\u0027initiator\u0027]]\" and explicitly pass in what you want?\n\nI know it seems really minor, and it probably is; but we have quite a few of these places in the code now where people wanted to introduce \"flexibility\" in the parameters and the result is it makes the code difficult to follow and maintain.","commit_id":"e06006106adff6bd49bc5c0dec5ca9a342569997"},{"author":{"_account_id":21309,"name":"Jay Mehta","email":"jay87.mehta@gmail.com","username":"Jay"},"change_message_id":"6afb08eb52611908e981603b39f18fb001818212","unresolved":false,"context_lines":[{"line_number":503,"context_line":"        used by 3PAR."},{"line_number":504,"context_line":"        \"\"\""},{"line_number":505,"context_line":"        # first search for an existing host"},{"line_number":506,"context_line":"        host_found \u003d None"},{"line_number":507,"context_line":"        if isinstance(iscsi_iqn, six.string_types):"},{"line_number":508,"context_line":"            iqn \u003d [iscsi_iqn]"},{"line_number":509,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7aa08908_2a4ab3ac","line":506,"range":{"start_line":506,"start_character":0,"end_line":506,"end_character":25},"in_reply_to":"7aa08908_b184615d","updated":"2016-06-10 18:51:20.000000000","message":"Done","commit_id":"e06006106adff6bd49bc5c0dec5ca9a342569997"},{"author":{"_account_id":21309,"name":"Jay Mehta","email":"jay87.mehta@gmail.com","username":"Jay"},"change_message_id":"6a75dc49113b018388127d4227895c739c549536","unresolved":false,"context_lines":[{"line_number":503,"context_line":"        used by 3PAR."},{"line_number":504,"context_line":"        \"\"\""},{"line_number":505,"context_line":"        # first search for an existing host"},{"line_number":506,"context_line":"        host_found \u003d None"},{"line_number":507,"context_line":"        if isinstance(iscsi_iqn, six.string_types):"},{"line_number":508,"context_line":"            iqn \u003d [iscsi_iqn]"},{"line_number":509,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7aa08908_4520d349","line":506,"range":{"start_line":506,"start_character":0,"end_line":506,"end_character":25},"in_reply_to":"7aa08908_b184615d","updated":"2016-06-10 18:47:51.000000000","message":"You are right with the connector that i can do [connector[\u0027initiator\u0027]] as it has a string value like \u0027initiator\u0027: u\u0027iqn.1993-08.org.debian:01:d9b12fc8719b\u0027.\nBut since the checks are already in place, I preferred to keep it and reposition it in the code place correctly.\n\nI will force in create_host() to pass it as [connector[\u0027initiator\u0027]] in my next patch set. And I will remove the type checking and conversion code.\n\nThanks for pointing out.","commit_id":"e06006106adff6bd49bc5c0dec5ca9a342569997"}]}