)]}'
{"os_brick/initiator/connectors/nvmeof.py":[{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"da2cc29fe85e41cb32bf52f42f90e461cc1fe89e","unresolved":true,"context_lines":[{"line_number":117,"context_line":"        all_nvme_devices \u003d self._get_nvme_devices()"},{"line_number":118,"context_line":"        LOG.debug(\"all_nvme_devices are %(all_nvme_devices)s\","},{"line_number":119,"context_line":"                  {\u0027all_nvme_devices\u0027: all_nvme_devices})"},{"line_number":120,"context_line":"        path \u003d set(all_nvme_devices) - set(current_nvme_devices)"},{"line_number":121,"context_line":"        if not path:"},{"line_number":122,"context_line":"            raise exception.VolumePathsNotFound()"},{"line_number":123,"context_line":"        return list(path)"}],"source_content_type":"text/x-python","patch_set":6,"id":"e77c4529_ee257503","side":"PARENT","line":120,"updated":"2021-01-30 03:22:18.000000000","message":"This is a definite issue for connect_volume which will have all sorts of problems with concurrency or any proper scale.\n\nThis is being fixed by the new connector.","commit_id":"ec70b4092f649d933322820e3003269560df7af9"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"ecb214b3baee864ccd372cab5c8cc83126758a44","unresolved":true,"context_lines":[{"line_number":50,"context_line":"        return \u0027/dev/\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def get_volume_paths(self, connection_properties):"},{"line_number":53,"context_line":"        return []"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    @staticmethod"},{"line_number":56,"context_line":"    def get_connector_properties(root_helper, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":6,"id":"0285261f_56c5ca0e","line":53,"updated":"2021-02-03 14:38:45.000000000","message":"Why can\u0027t you return any volume paths?  This should be populated if at all possible","commit_id":"664699ecc31565f8b6c2dfd8435c5bf8b8fb8b10"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"539882b94b9083eb78295e9858e932d061ab49ec","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        return \u0027/dev/\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def get_volume_paths(self, connection_properties):"},{"line_number":53,"context_line":"        return []"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    @staticmethod"},{"line_number":56,"context_line":"    def get_connector_properties(root_helper, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":6,"id":"f20e94b5_8eb8ae8e","line":53,"in_reply_to":"0285261f_56c5ca0e","updated":"2021-02-03 15:27:06.000000000","message":"Thank you for bringing this up! I have code we are testing with now that populatse this. We just have not seen any improved behavior with it, I would need to read into the code invoking it to get a better understanding. But anyway it is implemented and I just tested with this change to be sure and will commit it now.","commit_id":"664699ecc31565f8b6c2dfd8435c5bf8b8fb8b10"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b7354bb14cb7542ea7da68e28ba598ecb7b1e63e","unresolved":true,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def get_volume_paths(self, connection_properties):"},{"line_number":53,"context_line":"        device_path \u003d connection_properties.get(\u0027device_path\u0027)"},{"line_number":54,"context_line":"        if (device_path):"},{"line_number":55,"context_line":"            return [device_path]"},{"line_number":56,"context_line":"        volume_replicas \u003d connection_properties.get(\u0027volume_replicas\u0027)"},{"line_number":57,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"56ced789_a451b285","line":54,"range":{"start_line":54,"start_character":11,"end_line":54,"end_character":24},"updated":"2021-02-05 23:32:28.000000000","message":"nit: don\u0027t need the parentheses","commit_id":"7a8eb123ce193ddb5001acc62f08b50b4bfc9d41"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"43eb39233fa2ee2c7f3b45e1f716ad625a281d4a","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def get_volume_paths(self, connection_properties):"},{"line_number":53,"context_line":"        device_path \u003d connection_properties.get(\u0027device_path\u0027)"},{"line_number":54,"context_line":"        if (device_path):"},{"line_number":55,"context_line":"            return [device_path]"},{"line_number":56,"context_line":"        volume_replicas \u003d connection_properties.get(\u0027volume_replicas\u0027)"},{"line_number":57,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"1be51903_d97254d3","line":54,"range":{"start_line":54,"start_character":11,"end_line":54,"end_character":24},"in_reply_to":"56ced789_a451b285","updated":"2021-02-06 06:13:10.000000000","message":"Done","commit_id":"7a8eb123ce193ddb5001acc62f08b50b4bfc9d41"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b7354bb14cb7542ea7da68e28ba598ecb7b1e63e","unresolved":true,"context_lines":[{"line_number":94,"context_line":"            return lines.split(\u0027\\n\u0027)[0]"},{"line_number":95,"context_line":"        except putils.ProcessExecutionError as e:"},{"line_number":96,"context_line":"            LOG.warning("},{"line_number":97,"context_line":"                \"Process exection error in _get_host_uuid: %s\" % str(e))"},{"line_number":98,"context_line":"            return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def _get_host_nqn(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"e1138d97_487bfa87","line":97,"range":{"start_line":97,"start_character":25,"end_line":97,"end_character":33},"updated":"2021-02-05 23:32:28.000000000","message":"type: execution","commit_id":"7a8eb123ce193ddb5001acc62f08b50b4bfc9d41"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"43eb39233fa2ee2c7f3b45e1f716ad625a281d4a","unresolved":false,"context_lines":[{"line_number":94,"context_line":"            return lines.split(\u0027\\n\u0027)[0]"},{"line_number":95,"context_line":"        except putils.ProcessExecutionError as e:"},{"line_number":96,"context_line":"            LOG.warning("},{"line_number":97,"context_line":"                \"Process exection error in _get_host_uuid: %s\" % str(e))"},{"line_number":98,"context_line":"            return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def _get_host_nqn(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"b083dc1b_072f0c81","line":97,"range":{"start_line":97,"start_character":25,"end_line":97,"end_character":33},"in_reply_to":"e1138d97_487bfa87","updated":"2021-02-06 06:13:10.000000000","message":"Done","commit_id":"7a8eb123ce193ddb5001acc62f08b50b4bfc9d41"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b7354bb14cb7542ea7da68e28ba598ecb7b1e63e","unresolved":true,"context_lines":[{"line_number":163,"context_line":"        return {\u0027type\u0027: \u0027block\u0027, \u0027path\u0027: device_path}"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    @utils.trace"},{"line_number":166,"context_line":"    @synchronized(\u0027connect_volume\u0027)"},{"line_number":167,"context_line":"    def disconnect_volume(self, connection_properties, device_info,"},{"line_number":168,"context_line":"                          force\u003dFalse, ignore_errors\u003dFalse):"},{"line_number":169,"context_line":"        device_path \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"78995743_98325f32","line":166,"range":{"start_line":166,"start_character":19,"end_line":166,"end_character":34},"updated":"2021-02-05 23:32:28.000000000","message":"this was a good catch","commit_id":"7a8eb123ce193ddb5001acc62f08b50b4bfc9d41"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"43eb39233fa2ee2c7f3b45e1f716ad625a281d4a","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        return {\u0027type\u0027: \u0027block\u0027, \u0027path\u0027: device_path}"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    @utils.trace"},{"line_number":166,"context_line":"    @synchronized(\u0027connect_volume\u0027)"},{"line_number":167,"context_line":"    def disconnect_volume(self, connection_properties, device_info,"},{"line_number":168,"context_line":"                          force\u003dFalse, ignore_errors\u003dFalse):"},{"line_number":169,"context_line":"        device_path \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"a5e14c9b_920156e7","line":166,"range":{"start_line":166,"start_character":19,"end_line":166,"end_character":34},"in_reply_to":"78995743_98325f32","updated":"2021-02-06 06:13:10.000000000","message":"Ack","commit_id":"7a8eb123ce193ddb5001acc62f08b50b4bfc9d41"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ae988a4fd1d4b05049e42e91700110ef022e5a24","unresolved":true,"context_lines":[{"line_number":104,"context_line":"                f.close()"},{"line_number":105,"context_line":"        except IOError:"},{"line_number":106,"context_line":"            try:"},{"line_number":107,"context_line":"                out, err \u003d self.execute("},{"line_number":108,"context_line":"                    \u0027nvme\u0027, \u0027gen-hostuuid\u0027,"},{"line_number":109,"context_line":"                    root_helper\u003dself._root_helper, run_as_root\u003dTrue)"},{"line_number":110,"context_line":"                host_nqn \u003d out.strip()"}],"source_content_type":"text/x-python","patch_set":9,"id":"a08f3d2f_7e7e6feb","line":107,"range":{"start_line":107,"start_character":32,"end_line":107,"end_character":39},"updated":"2021-02-11 16:23:44.000000000","message":"This should be _execute\n\nIt\u0027s causing a bunch of \"Could not generate host nqn: \u0027NVMeOFConnector\u0027 object has no attribute \u0027execute\u0027\" in http://publiclogs.emc.com/75/768575/9/check/DellEMC_XtremIO_FC_os-brick/9bcacf9/DellEMC_XtremIO_FC_os-brick/81/logs/screen-n-cpu.txt.gz","commit_id":"72e9ecac40e08c281945b1f55666bfc5cb1bc58b"}],"os_brick/tests/initiator/connectors/test_nvmeof.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d98a3ccf12ecf4c30227dca65d8e6c97fda4119b","unresolved":true,"context_lines":[{"line_number":45,"context_line":"    @mock.patch.object(nvmeof.NVMeOFConnector, \u0027_get_host_nqn\u0027,"},{"line_number":46,"context_line":"                       return_value\u003d\u0027fakenqn\u0027)"},{"line_number":47,"context_line":"    def test_get_connector_properties_without_sysuuid("},{"line_number":48,"context_line":"            self, mock_uuid, mock_nqn):"},{"line_number":49,"context_line":"        props \u003d self.connector.get_connector_properties(\u0027sudo\u0027)"},{"line_number":50,"context_line":"        expected_props \u003d {\u0027nqn\u0027: \u0027fakenqn\u0027}"},{"line_number":51,"context_line":"        self.assertEqual(expected_props, props)"}],"source_content_type":"text/x-python","patch_set":6,"id":"6c111862_a58265c3","line":48,"range":{"start_line":48,"start_character":18,"end_line":48,"end_character":37},"updated":"2021-01-29 22:18:58.000000000","message":"it\u0027s not affecting this test because you don\u0027t refer to the mocks inside the test, but the order of arguments should go bottom to top to refer to the objects mocked in the decorators (i.e., this should be \u0027mock_nqn, mock_uuid\u0027).","commit_id":"664699ecc31565f8b6c2dfd8435c5bf8b8fb8b10"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"da2cc29fe85e41cb32bf52f42f90e461cc1fe89e","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    @mock.patch.object(nvmeof.NVMeOFConnector, \u0027_get_host_nqn\u0027,"},{"line_number":46,"context_line":"                       return_value\u003d\u0027fakenqn\u0027)"},{"line_number":47,"context_line":"    def test_get_connector_properties_without_sysuuid("},{"line_number":48,"context_line":"            self, mock_uuid, mock_nqn):"},{"line_number":49,"context_line":"        props \u003d self.connector.get_connector_properties(\u0027sudo\u0027)"},{"line_number":50,"context_line":"        expected_props \u003d {\u0027nqn\u0027: \u0027fakenqn\u0027}"},{"line_number":51,"context_line":"        self.assertEqual(expected_props, props)"}],"source_content_type":"text/x-python","patch_set":6,"id":"f7adf3f9_8c8e3c12","line":48,"range":{"start_line":48,"start_character":18,"end_line":48,"end_character":37},"in_reply_to":"6c111862_a58265c3","updated":"2021-01-30 03:22:18.000000000","message":"Done","commit_id":"664699ecc31565f8b6c2dfd8435c5bf8b8fb8b10"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b7354bb14cb7542ea7da68e28ba598ecb7b1e63e","unresolved":true,"context_lines":[{"line_number":13,"context_line":"from unittest import mock"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import ddt"},{"line_number":16,"context_line":"# from oslo_concurrency import processutils as putils"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"# from os_brick import exception"},{"line_number":19,"context_line":"from os_brick.initiator.connectors import nvmeof"},{"line_number":20,"context_line":"from os_brick.initiator import linuxscsi"},{"line_number":21,"context_line":"from os_brick.tests.initiator import test_connector"}],"source_content_type":"text/x-python","patch_set":8,"id":"20bea442_ca37322a","line":18,"range":{"start_line":16,"start_character":0,"end_line":18,"end_character":32},"updated":"2021-02-05 23:32:28.000000000","message":"You should remove these commented-out lines.  On the other hand, it looks like the old tests were checking for some exception conditions that the current tests are not.  It would be a good idea to be sure that those really don\u0027t need to be tested any more.  The code coverage report for nvmeof.py is indicating 39% coverage; you can look at the report to see some areas that could be addressed:\nhttps://db1b1a0d863b2be8e4df-f6d9d58f0487401586e66f66be7f6106.ssl.cf2.rackcdn.com/768575/8/check/os-brick-code-coverage/1f2a082/cover/os_brick_initiator_connectors_nvmeof_py.html\n\nThe old code had 87% coverage.  Of course, it also had at least one bug that you\u0027ve fixed.  We don\u0027t have a specific coverage target for the project, but 39% is pretty low.  When you look at the report, you\u0027ll notice that some of the new functions you\u0027ve added aren\u0027t covered at all.  So I think a few more tests are in order.","commit_id":"7a8eb123ce193ddb5001acc62f08b50b4bfc9d41"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"43eb39233fa2ee2c7f3b45e1f716ad625a281d4a","unresolved":true,"context_lines":[{"line_number":13,"context_line":"from unittest import mock"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import ddt"},{"line_number":16,"context_line":"# from oslo_concurrency import processutils as putils"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"# from os_brick import exception"},{"line_number":19,"context_line":"from os_brick.initiator.connectors import nvmeof"},{"line_number":20,"context_line":"from os_brick.initiator import linuxscsi"},{"line_number":21,"context_line":"from os_brick.tests.initiator import test_connector"}],"source_content_type":"text/x-python","patch_set":8,"id":"b9f64d9e_8552de50","line":18,"range":{"start_line":16,"start_character":0,"end_line":18,"end_character":32},"in_reply_to":"20bea442_ca37322a","updated":"2021-02-06 06:13:10.000000000","message":"Thank you for this comment and the very useful pointer to the test coverage report.\n\nFor now I am adding a few simple tests of methods that were not covered yet.\n\nWe are actually planning on increasing this nvmeof connector code coverage to be as high as our driver, though we would like to ask to add all this extra extensive coverage in a follow up patch. Once the review for this connector is complete and maintainers are happy with the design and implementation, we will complete the more extensive unit test coverage hopefully within several days.","commit_id":"7a8eb123ce193ddb5001acc62f08b50b4bfc9d41"}]}