)]}'
{"os_brick/initiator/linuxfc.py":[{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"0fde7fb433cfa0e853b23260c81671281573b5ea","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    def _get_hba_channel_scsi_target(self, hba):"},{"line_number":31,"context_line":"        \"\"\"Try to get the HBA channel and SCSI target for an HBA."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        Works for Fibre Channel storage servers that implement a single WWNN"},{"line_number":34,"context_line":"        for all ports,"},{"line_number":35,"context_line":"        \"\"\""},{"line_number":36,"context_line":"        # Leave only the number from the host_device field (ie: host6)"},{"line_number":37,"context_line":"        host_device \u003d hba[\u0027host_device\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_5e92d84c","line":34,"range":{"start_line":33,"start_character":8,"end_line":34,"end_character":22},"updated":"2016-08-01 21:28:31.000000000","message":"This comment is kind of concerning... I think it means the fc target, right? We should maybe elaborate that in cases where they don\u0027t have a single WWN it\u0027s totally fine. We then just won\u0027t read the file to get the channel/id/etc, and will scan the host with wildcards and LUN. At first glance I was like \"oh crap, so my backend just won\u0027t work with this?\", could save some minor heart attacks with a little more explanation.","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"319026834b529827ba985928b3fb72adfc14c33d","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    def _get_hba_channel_scsi_target(self, hba):"},{"line_number":31,"context_line":"        \"\"\"Try to get the HBA channel and SCSI target for an HBA."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        Works for Fibre Channel storage servers that implement a single WWNN"},{"line_number":34,"context_line":"        for all ports,"},{"line_number":35,"context_line":"        \"\"\""},{"line_number":36,"context_line":"        # Leave only the number from the host_device field (ie: host6)"},{"line_number":37,"context_line":"        host_device \u003d hba[\u0027host_device\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_2fafbb7e","line":34,"range":{"start_line":33,"start_character":8,"end_line":34,"end_character":22},"in_reply_to":"bacf61ea_0f72b7ad","updated":"2016-08-02 17:45:25.000000000","message":"If it gave you a scare, others will have the same issue with the docstring, so it makes a lot of sense to update it and make it explicit.","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"606e1408868c725c5ee54c67aefd78be249899bd","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    def _get_hba_channel_scsi_target(self, hba):"},{"line_number":31,"context_line":"        \"\"\"Try to get the HBA channel and SCSI target for an HBA."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        Works for Fibre Channel storage servers that implement a single WWNN"},{"line_number":34,"context_line":"        for all ports,"},{"line_number":35,"context_line":"        \"\"\""},{"line_number":36,"context_line":"        # Leave only the number from the host_device field (ie: host6)"},{"line_number":37,"context_line":"        host_device \u003d hba[\u0027host_device\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_0f72b7ad","line":34,"range":{"start_line":33,"start_character":8,"end_line":34,"end_character":22},"in_reply_to":"bacf61ea_49caef6f","updated":"2016-08-02 17:38:35.000000000","message":"Haha, yea I know I\u0027m probably being overly paranoid about it (and lazy since its like 10 lines of code to know what happens).\n\nI think the main distinction I like to have made very clear is for these os-brick commands knowing if it will try and gracefully fail the overall attach process, or maybe some feature of it disabled, or try and then try something else thats a perf hit, etc etc. We do lots of trying so its always helpful to know the consequences of failing (without having to read the code)","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fc5a414e4e37ed2a4fba8334a0cbedbdce608bc3","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    def _get_hba_channel_scsi_target(self, hba):"},{"line_number":31,"context_line":"        \"\"\"Try to get the HBA channel and SCSI target for an HBA."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        Works for Fibre Channel storage servers that implement a single WWNN"},{"line_number":34,"context_line":"        for all ports,"},{"line_number":35,"context_line":"        \"\"\""},{"line_number":36,"context_line":"        # Leave only the number from the host_device field (ie: host6)"},{"line_number":37,"context_line":"        host_device \u003d hba[\u0027host_device\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_49caef6f","line":34,"range":{"start_line":33,"start_character":8,"end_line":34,"end_character":22},"in_reply_to":"bacf61ea_5e92d84c","updated":"2016-08-02 17:21:17.000000000","message":"Well, the first word in the docstring is \"Try\" for that reason; meaning that it is acceptable to fail, but I\u0027ll update it to make it more explicit.\n\nAnd looking at the rescan_hosts (where it is used) you\u0027ll see that I explicitly say this L55 and L57\n\nYeah, I\u0027m talking about the FC target.","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"0fde7fb433cfa0e853b23260c81671281573b5ea","unresolved":false,"context_lines":[{"line_number":39,"context_line":"            host_device \u003d host_device[4:]"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"        path \u003d \u0027/sys/class/fc_transport/target%s:\u0027 % host_device"},{"line_number":42,"context_line":"        cmd \u003d \u0027grep %(wwnn)s %(path)s*/node_name\u0027 % {\u0027wwnn\u0027: hba[\u0027node_name\u0027],"},{"line_number":43,"context_line":"                                                     \u0027path\u0027: path}"},{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            out, _err \u003d self._execute(cmd)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_5e293889","line":42,"updated":"2016-08-01 21:28:31.000000000","message":"Seems kind of odd to be calling grep from python code. I\u0027m guessing its to expand the wildcard \u0027*\u0027 path, right? Or do we need to do this as root too?\n\n\nIf we split this up into a couple steps like listing the files for the dir, and then reading the node_name for each of looking for our wwn, we could differentiate between some other error parsing or running the command and systems that have multiple WWNN\u0027s and won\u0027t have files show up here at all. Not sure what kind of perf penalties we pay to do that though.","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fc5a414e4e37ed2a4fba8334a0cbedbdce608bc3","unresolved":false,"context_lines":[{"line_number":39,"context_line":"            host_device \u003d host_device[4:]"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"        path \u003d \u0027/sys/class/fc_transport/target%s:\u0027 % host_device"},{"line_number":42,"context_line":"        cmd \u003d \u0027grep %(wwnn)s %(path)s*/node_name\u0027 % {\u0027wwnn\u0027: hba[\u0027node_name\u0027],"},{"line_number":43,"context_line":"                                                     \u0027path\u0027: path}"},{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            out, _err \u003d self._execute(cmd)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_c9637f40","line":42,"in_reply_to":"bacf61ea_5e293889","updated":"2016-08-02 17:21:17.000000000","message":"If we change this to do it in python it will require more lines of code and maybe more error checking.  If you really thing this is better done in Python I\u0027ll change it.","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"606e1408868c725c5ee54c67aefd78be249899bd","unresolved":false,"context_lines":[{"line_number":39,"context_line":"            host_device \u003d host_device[4:]"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"        path \u003d \u0027/sys/class/fc_transport/target%s:\u0027 % host_device"},{"line_number":42,"context_line":"        cmd \u003d \u0027grep %(wwnn)s %(path)s*/node_name\u0027 % {\u0027wwnn\u0027: hba[\u0027node_name\u0027],"},{"line_number":43,"context_line":"                                                     \u0027path\u0027: path}"},{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            out, _err \u003d self._execute(cmd)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_2fa4db06","line":42,"in_reply_to":"bacf61ea_c9637f40","updated":"2016-08-02 17:38:35.000000000","message":"Thats fair, i\u0027m ok with leaving it as grep.","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"0fde7fb433cfa0e853b23260c81671281573b5ea","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            out, _err \u003d self._execute(cmd)"},{"line_number":46,"context_line":"            return [line.split(\u0027/\u0027)[4].split(\u0027:\u0027)[1:]"},{"line_number":47,"context_line":"                    for line in out.split(\u0027\\n\u0027) if line.startswith(path)]"},{"line_number":48,"context_line":"        except Exception as exc:"},{"line_number":49,"context_line":"            LOG.warning(_LW(\u0027Could not get HBA channel and SCSI target ID, \u0027"},{"line_number":50,"context_line":"                            \u0027reason: %s\u0027), exc)"},{"line_number":51,"context_line":"            return None"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_9c29f9e8","line":48,"updated":"2016-08-01 21:28:31.000000000","message":"Do we really want to catch all exceptions? Like a parsing or exec failure we do, but some other unexpected one we might want to raise up the stack.","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fc5a414e4e37ed2a4fba8334a0cbedbdce608bc3","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            out, _err \u003d self._execute(cmd)"},{"line_number":46,"context_line":"            return [line.split(\u0027/\u0027)[4].split(\u0027:\u0027)[1:]"},{"line_number":47,"context_line":"                    for line in out.split(\u0027\\n\u0027) if line.startswith(path)]"},{"line_number":48,"context_line":"        except Exception as exc:"},{"line_number":49,"context_line":"            LOG.warning(_LW(\u0027Could not get HBA channel and SCSI target ID, \u0027"},{"line_number":50,"context_line":"                            \u0027reason: %s\u0027), exc)"},{"line_number":51,"context_line":"            return None"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_29e7bbad","line":48,"in_reply_to":"bacf61ea_9c29f9e8","updated":"2016-08-02 17:21:17.000000000","message":"Since this is not required information for the rescan to succeed I don\u0027t want to fail just because we can\u0027t get it, it seems too disruptive.  Though it may be a good idea to raise the log level to errorr.","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"6302df134d3bc7c6c6fc2c642d49d782afd62fc0","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        path \u003d \u0027/sys/class/fc_transport/target%s:\u0027 % host_device"},{"line_number":43,"context_line":"        cmd \u003d \u0027grep %(wwnn)s %(path)s*/node_name\u0027 % {\u0027wwnn\u0027: hba[\u0027node_name\u0027],"},{"line_number":44,"context_line":"                                                     \u0027path\u0027: path}"},{"line_number":45,"context_line":"        try:"},{"line_number":46,"context_line":"            out, _err \u003d self._execute(cmd)"},{"line_number":47,"context_line":"            return [line.split(\u0027/\u0027)[4].split(\u0027:\u0027)[1:]"}],"source_content_type":"text/x-python","patch_set":3,"id":"bacf61ea_88c82576","line":44,"updated":"2016-08-03 17:11:47.000000000","message":"the cmd needs to be a list [], not a string.","commit_id":"28a4d55a0a465ac36ed012a2d634cb64e8f5d599"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"2266ddaa8cdcd9411ffcb59ed48d48f47d49a79d","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        path \u003d \u0027/sys/class/fc_transport/target%s:\u0027 % host_device"},{"line_number":43,"context_line":"        cmd \u003d \u0027grep %(wwnn)s %(path)s*/node_name\u0027 % {\u0027wwnn\u0027: hba[\u0027node_name\u0027],"},{"line_number":44,"context_line":"                                                     \u0027path\u0027: path}"},{"line_number":45,"context_line":"        try:"},{"line_number":46,"context_line":"            out, _err \u003d self._execute(cmd)"},{"line_number":47,"context_line":"            return [line.split(\u0027/\u0027)[4].split(\u0027:\u0027)[1:]"}],"source_content_type":"text/x-python","patch_set":3,"id":"bacf61ea_c7bf8afa","line":44,"in_reply_to":"bacf61ea_88c82576","updated":"2016-08-04 08:45:13.000000000","message":"No it shouldn\u0027t, this is correct.\n\nYou can see it in the execute method declarations [1][2] and in many other cases in os-brick, one such cases is on L77 in this same file.\n\n[1]: https://github.com/openstack/os-brick/blob/master/os_brick/privileged/rootwrap.py#L50\n[2]: https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/processutils.py#L181","commit_id":"28a4d55a0a465ac36ed012a2d634cb64e8f5d599"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"50024681b7897b417365d365d2ec6c391eec1ded","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        path \u003d \u0027/sys/class/fc_transport/target%s:\u0027 % host_device"},{"line_number":43,"context_line":"        cmd \u003d \u0027grep %(wwnn)s %(path)s*/node_name\u0027 % {\u0027wwnn\u0027: hba[\u0027node_name\u0027],"},{"line_number":44,"context_line":"                                                     \u0027path\u0027: path}"},{"line_number":45,"context_line":"        try:"},{"line_number":46,"context_line":"            out, _err \u003d self._execute(cmd)"},{"line_number":47,"context_line":"            return [line.split(\u0027/\u0027)[4].split(\u0027:\u0027)[1:]"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_ea639018","line":44,"in_reply_to":"bacf61ea_c7bf8afa","updated":"2016-08-09 16:16:44.000000000","message":"you are right, it\u0027s multiple strings, not a list.","commit_id":"28a4d55a0a465ac36ed012a2d634cb64e8f5d599"}],"os_brick/tests/initiator/test_linuxfc.py":[{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"0fde7fb433cfa0e853b23260c81671281573b5ea","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        self.cmds.append(\" \".join(cmd))"},{"line_number":35,"context_line":"        return \"\", None"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def test_rescan_hosts(self):"},{"line_number":38,"context_line":"        # We check that we try to get the HBA channel and SCSI target"},{"line_number":39,"context_line":"        execute_results \u003d ("},{"line_number":40,"context_line":"            (\u0027/sys/class/fc_transport/target10:2:3/node_name:\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_bfeb0fdb","line":37,"updated":"2016-08-01 21:28:31.000000000","message":"Can we add a test for the case where calling execute raises an exception (causing the wild-cards to be used)?","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fc5a414e4e37ed2a4fba8334a0cbedbdce608bc3","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        self.cmds.append(\" \".join(cmd))"},{"line_number":35,"context_line":"        return \"\", None"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def test_rescan_hosts(self):"},{"line_number":38,"context_line":"        # We check that we try to get the HBA channel and SCSI target"},{"line_number":39,"context_line":"        execute_results \u003d ("},{"line_number":40,"context_line":"            (\u0027/sys/class/fc_transport/target10:2:3/node_name:\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_09a3d750","line":37,"in_reply_to":"bacf61ea_bfeb0fdb","updated":"2016-08-02 17:21:17.000000000","message":"Sure! I\u0027ll add it","commit_id":"9fae7ea2d716cbe283818f8d7c1c27738a8e24f2"}]}