)]}'
{"ironic_python_agent/extensions/image.py":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"a4e26963a5ce0cd6c7c41ae4bea73a62073e7e43","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        if efi_partition:"},{"line_number":101,"context_line":"            if not os.path.exists(efi_partition_mount_point):"},{"line_number":102,"context_line":"                os.makedirs(efi_partition_mount_point)"},{"line_number":103,"context_line":"            utils.execute(\u0027mount\u0027, efi_partition, efi_partition_mount_point)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        binary_name \u003d \"grub\""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_50d9781f","line":102,"updated":"2017-11-17 21:28:17.000000000","message":"This is creating the directory.","commit_id":"dd6056d7cbbd48a2573ca1f49445b429e36f2834"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"a4e26963a5ce0cd6c7c41ae4bea73a62073e7e43","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        if efi_partition:"},{"line_number":101,"context_line":"            if not os.path.exists(efi_partition_mount_point):"},{"line_number":102,"context_line":"                os.makedirs(efi_partition_mount_point)"},{"line_number":103,"context_line":"            utils.execute(\u0027mount\u0027, efi_partition, efi_partition_mount_point)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        binary_name \u003d \"grub\""},{"line_number":106,"context_line":"        if os.path.exists(os.path.join(path, \u0027usr/sbin/grub2-install\u0027)):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_1009c094","line":103,"updated":"2017-11-17 21:28:17.000000000","message":"If this fails, then we are still going to try to unmount the directory.","commit_id":"dd6056d7cbbd48a2573ca1f49445b429e36f2834"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"a4e26963a5ce0cd6c7c41ae4bea73a62073e7e43","unresolved":false,"context_lines":[{"line_number":101,"context_line":"            if not os.path.exists(efi_partition_mount_point):"},{"line_number":102,"context_line":"                os.makedirs(efi_partition_mount_point)"},{"line_number":103,"context_line":"            utils.execute(\u0027mount\u0027, efi_partition, efi_partition_mount_point)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        binary_name \u003d \"grub\""},{"line_number":106,"context_line":"        if os.path.exists(os.path.join(path, \u0027usr/sbin/grub2-install\u0027)):"},{"line_number":107,"context_line":"            binary_name \u003d \"grub2\""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_b00f8c86","line":104,"updated":"2017-11-17 21:28:17.000000000","message":"What if we add:\n\n mounted_efi \u003d True\n\n?","commit_id":"dd6056d7cbbd48a2573ca1f49445b429e36f2834"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"dd227d57ac49656115ddb5749b4556b8f36a979e","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        # If umount fails for efi partition, then we cannot be sure that all"},{"line_number":156,"context_line":"        # the changes were written back to the filesystem."},{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            if efi_partition and os.path.exists(efi_partition_mount_point):"},{"line_number":159,"context_line":"                utils.execute(\u0027umount\u0027, efi_partition_mount_point, attempts\u003d3,"},{"line_number":160,"context_line":"                              delay_on_retry\u003dTrue)"},{"line_number":161,"context_line":"        except processutils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_3a31405b","line":158,"updated":"2017-11-17 15:54:27.000000000","message":"I think it would actually be better to check to see if it is mounted instead of there is a partition and a path. I think it would likely future-proof this section of code a little better, but that is jut my opinion. This should work just fine for most cases.","commit_id":"dd6056d7cbbd48a2573ca1f49445b429e36f2834"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"a4e26963a5ce0cd6c7c41ae4bea73a62073e7e43","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        # If umount fails for efi partition, then we cannot be sure that all"},{"line_number":156,"context_line":"        # the changes were written back to the filesystem."},{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            if efi_partition and os.path.exists(efi_partition_mount_point):"},{"line_number":159,"context_line":"                utils.execute(\u0027umount\u0027, efi_partition_mount_point, attempts\u003d3,"},{"line_number":160,"context_line":"                              delay_on_retry\u003dTrue)"},{"line_number":161,"context_line":"        except processutils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_504738bd","line":158,"in_reply_to":"1f485f77_3a31405b","updated":"2017-11-17 21:28:17.000000000","message":"I\u0027m not sure how this fixes anything?\n\nSince the directory should exist even if the mount failed.\n\nPersonally I think the try block starting at line 86 is way too big. Since we aren\u0027t sure what failed.","commit_id":"dd6056d7cbbd48a2573ca1f49445b429e36f2834"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b474d74cd2ce867fdbcb65154d5e5c3620bdd5b7","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        # If umount fails for efi partition, then we cannot be sure that all"},{"line_number":156,"context_line":"        # the changes were written back to the filesystem."},{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            if efi_partition and os.path.exists(efi_partition_mount_point):"},{"line_number":159,"context_line":"                utils.execute(\u0027umount\u0027, efi_partition_mount_point, attempts\u003d3,"},{"line_number":160,"context_line":"                              delay_on_retry\u003dTrue)"},{"line_number":161,"context_line":"        except processutils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_2c919c53","line":158,"in_reply_to":"1f485f77_504738bd","updated":"2017-11-20 10:33:01.000000000","message":"Well, this fixes the problem in question :) but I agree there is another potential problem we could fix as well.","commit_id":"dd6056d7cbbd48a2573ca1f49445b429e36f2834"}],"ironic_python_agent/tests/unit/extensions/test_image.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"0d4da26b7b4c666f7580d4219d2e079fc23e62de","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                          efi_system_part_uuid\u003dself.fake_efi_system_part_uuid)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        mock_execute.assert_called_once_with(\u0027mount\u0027, \u0027/dev/fake2\u0027,"},{"line_number":232,"context_line":"                                             self.fake_dir)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    @mock.patch.object(image, \u0027_get_partition\u0027, autospec\u003dTrue)"},{"line_number":235,"context_line":"    def test__install_grub2_command_fail(self, mock_get_part_uuid,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_6dc8f054","line":232,"updated":"2017-11-20 22:20:01.000000000","message":"I am confused by how/what this is testing. If I understand it, the code will call utils.execute() several times, so how is it that this assert passes.\n\nEg, in images.py, L171 and L178. What am I missing?","commit_id":"9e7aa75f918b3f5ca10bbd783168005d53aad245"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"f83500e221d58ccefa2c648951f1bb2cebf6f495","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                          efi_system_part_uuid\u003dself.fake_efi_system_part_uuid)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        mock_execute.assert_called_once_with(\u0027mount\u0027, \u0027/dev/fake2\u0027,"},{"line_number":232,"context_line":"                                             self.fake_dir)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    @mock.patch.object(image, \u0027_get_partition\u0027, autospec\u003dTrue)"},{"line_number":235,"context_line":"    def test__install_grub2_command_fail(self, mock_get_part_uuid,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_e6314ba4","line":232,"in_reply_to":"ff82abbf_6dc8f054","updated":"2017-11-22 12:33:41.000000000","message":"OUCH. After John\u0027s execute() patch all assert_* methods are just mocks, they\u0027re not testing anything Oo I\u0027m not sure what to do.\n\nWe cannot use autospec with this approach, as autospec catches the spec of the Mock class, not of the original object.","commit_id":"9e7aa75f918b3f5ca10bbd783168005d53aad245"}]}