)]}'
{"os_brick/privileged/nvmeof.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d692884d6aa33dac7027ee07a9b641fa24713ecb","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Try to get existing nqn generated from dmi or systemd"},{"line_number":43,"context_line":"        try:"},{"line_number":44,"context_line":"            host_nqn, err \u003d rootwrap.custom_execute(\u0027nvme\u0027, \u0027show-hostnqn\u0027)"},{"line_number":45,"context_line":"            host_nqn \u003d host_nqn.strip()"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"        # This is different from OSError\u0027s ENOENT, which is missing nvme"}],"source_content_type":"text/x-python","patch_set":2,"id":"04753c36_3a6944a2","line":44,"range":{"start_line":44,"start_character":53,"end_line":44,"end_character":73},"updated":"2021-06-08 08:50:59.000000000","message":"I tried to run this command by omitting machine-id and it returns code 254. IIUC error codes lie in the range 1-255 and this will generate ProcessExecutionError exception right?\nIn that case we will raise but nvme gen-hostnqn still works without the machine-id","commit_id":"37d57c4306acc59dfe8f025040b7701dea2d304c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ad7eae9a4f73c786b158c2efb87ccaaf4130df25","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Try to get existing nqn generated from dmi or systemd"},{"line_number":43,"context_line":"        try:"},{"line_number":44,"context_line":"            host_nqn, err \u003d rootwrap.custom_execute(\u0027nvme\u0027, \u0027show-hostnqn\u0027)"},{"line_number":45,"context_line":"            host_nqn \u003d host_nqn.strip()"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"        # This is different from OSError\u0027s ENOENT, which is missing nvme"}],"source_content_type":"text/x-python","patch_set":2,"id":"1fb4b9e1_a3ee3dab","line":44,"range":{"start_line":44,"start_character":53,"end_line":44,"end_character":73},"in_reply_to":"04753c36_3a6944a2","updated":"2021-06-08 10:34:18.000000000","message":"I don\u0027t understand what you mean by \"run this command by omitting machine-id\".\nRegardless of the error, we catch it on L63 if the nvme cannot get it from the system or if there\u0027s any unexpected error so we don\u0027t fail.\n\nWhether the nvme attachment process will work or not without this information is irrelevant to this call, the important things are that get_connector_properties doesn\u0027t ever fail, that it logs things right, and that it doesn\u0027t return different information if there has been no changes to the system (which is what we were trying to fix).\n\nThe normal nvme attachment flow isn\u0027t currently using the nqn, so it will work regardless. The only driver that currently uses it is Kioxia, and if it cannot work without it then it should fail on the initialize_connection call of the Cinder driver, just like FC drivers fail if there is no HBAs reported from the attaching host.","commit_id":"37d57c4306acc59dfe8f025040b7701dea2d304c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"63bfdf2752b51aa89aebfff94fd0d851aa6018de","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Try to get existing nqn generated from dmi or systemd"},{"line_number":43,"context_line":"        try:"},{"line_number":44,"context_line":"            host_nqn, err \u003d rootwrap.custom_execute(\u0027nvme\u0027, \u0027show-hostnqn\u0027)"},{"line_number":45,"context_line":"            host_nqn \u003d host_nqn.strip()"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"        # This is different from OSError\u0027s ENOENT, which is missing nvme"}],"source_content_type":"text/x-python","patch_set":2,"id":"87314483_6f73e221","line":44,"range":{"start_line":44,"start_character":53,"end_line":44,"end_character":73},"in_reply_to":"1fb4b9e1_a3ee3dab","updated":"2021-06-08 12:43:01.000000000","message":"By omitting machine-id i meant, remove the /etc/machine-id file because i figured that is used to generate nqn when executing show-hostnqn command, the command errors out if /etc/nvme/hostnqn doesn\u0027t exist. (maybe there is more depth to it which I\u0027m unaware of)\nMy query was just about why do we fail on show-hostnqn (if error is anything except 2) and not run gen-hostnqn when gen-hostnqn could succeed and provide an nqn which now i realised makes sense and also you\u0027ve answered clearly. Thanks.","commit_id":"37d57c4306acc59dfe8f025040b7701dea2d304c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7d56abe0d14396cb61735e1b9d632e01096c6d72","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Try to get existing nqn generated from dmi or systemd"},{"line_number":43,"context_line":"        try:"},{"line_number":44,"context_line":"            host_nqn, err \u003d rootwrap.custom_execute(\u0027nvme\u0027, \u0027show-hostnqn\u0027)"},{"line_number":45,"context_line":"            host_nqn \u003d host_nqn.strip()"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"        # This is different from OSError\u0027s ENOENT, which is missing nvme"}],"source_content_type":"text/x-python","patch_set":2,"id":"07d1057f_bb77554a","line":44,"range":{"start_line":44,"start_character":53,"end_line":44,"end_character":73},"in_reply_to":"87314483_6f73e221","updated":"2021-06-09 19:28:52.000000000","message":"It should not return 254, it should be returning exit code 2.\nI\u0027ll reproduce it and try to find out what\u0027s going on...","commit_id":"37d57c4306acc59dfe8f025040b7701dea2d304c"}],"os_brick/tests/privileged/test_nvmeof.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"33df09bf5f4db817d45cdc860ea943ef81935cd4","unresolved":true,"context_lines":[{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        mock_open.assert_called_once_with(\u0027/etc/nvme/hostnqn\u0027, \u0027w\u0027)"},{"line_number":81,"context_line":"        stripped_hostnqn \u003d hostnqn.strip.return_value"},{"line_number":82,"context_line":"        mock_open.return_value.__enter__.return_value.write.\\"},{"line_number":83,"context_line":"            assert_called_once_with(stripped_hostnqn)"},{"line_number":84,"context_line":"        mock_chmod.assert_called_once_with(\u0027/etc/nvme/hostnqn\u0027, 0o644)"},{"line_number":85,"context_line":"        self.assertEqual(stripped_hostnqn, res)"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"d87883ae_a3a63cb7","line":83,"range":{"start_line":82,"start_character":0,"end_line":83,"end_character":53},"updated":"2021-06-03 13:10:33.000000000","message":"There\u0027s nothing wrong with what you have here, but if you change line 60 to\n\n  @mock.patch.object(builtins, \u0027open\u0027, new_callable\u003dmock.mock_open)\n\nthen this line can be simply\n\n  mock_open().write.assert_called_once_with(stripped_hostnqn)\n\n(same thing in the previous function)","commit_id":"174e18da854dfabfed9c4cd9641c2453e52f33a9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"94304efab305de33cc5ef08b224ee6baa4c9453f","unresolved":false,"context_lines":[{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        mock_open.assert_called_once_with(\u0027/etc/nvme/hostnqn\u0027, \u0027w\u0027)"},{"line_number":81,"context_line":"        stripped_hostnqn \u003d hostnqn.strip.return_value"},{"line_number":82,"context_line":"        mock_open.return_value.__enter__.return_value.write.\\"},{"line_number":83,"context_line":"            assert_called_once_with(stripped_hostnqn)"},{"line_number":84,"context_line":"        mock_chmod.assert_called_once_with(\u0027/etc/nvme/hostnqn\u0027, 0o644)"},{"line_number":85,"context_line":"        self.assertEqual(stripped_hostnqn, res)"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f2d3c69_48dee647","line":83,"range":{"start_line":82,"start_character":0,"end_line":83,"end_character":53},"in_reply_to":"d87883ae_a3a63cb7","updated":"2021-06-07 14:05:30.000000000","message":"Thanks, it\u0027s a lot more readable that way.","commit_id":"174e18da854dfabfed9c4cd9641c2453e52f33a9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"63bfdf2752b51aa89aebfff94fd0d851aa6018de","unresolved":true,"context_lines":[{"line_number":50,"context_line":"                                            exist_ok\u003dTrue)"},{"line_number":51,"context_line":"        mock_exec.assert_called_once_with(\u0027nvme\u0027, \u0027show-hostnqn\u0027)"},{"line_number":52,"context_line":"        mock_open.assert_called_once_with(\u0027/etc/nvme/hostnqn\u0027, \u0027w\u0027)"},{"line_number":53,"context_line":"        stripped_hostnqn \u003d hostnqn.strip.return_value"},{"line_number":54,"context_line":"        mock_open().write.assert_called_once_with(stripped_hostnqn)"},{"line_number":55,"context_line":"        mock_chmod.assert_called_once_with(\u0027/etc/nvme/hostnqn\u0027, 0o644)"},{"line_number":56,"context_line":"        self.assertEqual(stripped_hostnqn, res)"}],"source_content_type":"text/x-python","patch_set":2,"id":"987851ef_00b150df","line":53,"range":{"start_line":53,"start_character":27,"end_line":53,"end_character":53},"updated":"2021-06-08 12:43:01.000000000","message":"IIUC this will turn out to be a mock format call like \u003cMagicMock name\u003d\u0027mock.strip()\u0027 id\u003d\u0027140004674997712\u0027\u003e\n\nIt works but i would prefer setting a fake return value at the start like fake-nqn or anything (although it\u0027s not being printed anywhere)","commit_id":"37d57c4306acc59dfe8f025040b7701dea2d304c"}]}
