)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"3fa359b7f61fe5b245456038529492660313b7fd","unresolved":false,"context_lines":[{"line_number":17,"context_line":"to delete the vmdk and -flat vmdk files separately. This API does not"},{"line_number":18,"context_line":"suffer from the above-mentioned failure mode."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Fixes bug: 129994"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I786365847673e5192a21b654cba951b2e7a6f291"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"AAAATH%2F%2FKEE%3D","line":20,"updated":"2013-10-22 11:04:58.000000000","message":"You forgot a \u00272\u0027, also please use this format:\n\n  Closes-Bug: #1229994","commit_id":"1366324e53dfa25e1e7b917e46041008ad1dcc42"}],"nova/tests/virt/vmwareapi/test_vmwareapi.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8e3641d9bf165e8b0518e8ef33615d87142161ef","unresolved":false,"context_lines":[{"line_number":1133,"context_line":"                                 \u0027_delete_datastore_file\u0027)"},{"line_number":1134,"context_line":"        # Check calls for delete vmdk and -flat.vmdk pair"},{"line_number":1135,"context_line":"        for i in xrange(2):"},{"line_number":1136,"context_line":"            self.conn._vmops._delete_datastore_file("},{"line_number":1137,"context_line":"                    mox.IgnoreArg(), mox.IgnoreArg(),"},{"line_number":1138,"context_line":"                    mox.IgnoreArg()).AndReturn(None)"},{"line_number":1139,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAS3%2F%2FNtM%3D","line":1136,"updated":"2013-10-02 06:27:42.000000000","message":"is it possible to check the parameters here for the file names?","commit_id":"cc53e98bb2e9ec771e9ae6bb7171151d5d33efa1"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"2df5a6d9ec32156698307b27df7d700c193cb29f","unresolved":false,"context_lines":[{"line_number":1133,"context_line":"                                 \u0027_delete_datastore_file\u0027)"},{"line_number":1134,"context_line":"        # Check calls for delete vmdk and -flat.vmdk pair"},{"line_number":1135,"context_line":"        for i in xrange(2):"},{"line_number":1136,"context_line":"            self.conn._vmops._delete_datastore_file("},{"line_number":1137,"context_line":"                    mox.IgnoreArg(), mox.IgnoreArg(),"},{"line_number":1138,"context_line":"                    mox.IgnoreArg()).AndReturn(None)"},{"line_number":1139,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAS3%2F%2FLwc%3D","line":1136,"in_reply_to":"AAAAS3%2F%2FNtM%3D","updated":"2013-10-02 19:03:19.000000000","message":"Done","commit_id":"cc53e98bb2e9ec771e9ae6bb7171151d5d33efa1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"6525d7f09f4432d35fe7c3181a40ac100e7b199e","unresolved":false,"context_lines":[{"line_number":1142,"context_line":""},{"line_number":1143,"context_line":"    def test_snapshot_using_file_manager(self):"},{"line_number":1144,"context_line":"        self._create_vm()"},{"line_number":1145,"context_line":"        uuid_str \u003d str(uuid.uuid4())"},{"line_number":1146,"context_line":"        self.mox.StubOutWithMock(vmops.VMwareVMOps,"},{"line_number":1147,"context_line":"                                 \u0027_generate_random_vmdk_name\u0027)"},{"line_number":1148,"context_line":"        vmops.VMwareVMOps._generate_random_vmdk_name().AndReturn(uuid_str)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAS3%2F%2FKP8%3D","line":1145,"updated":"2013-10-03 07:54:21.000000000","message":"please use https://github.com/openstack/nova/blob/master/nova/openstack/common/uuidutils.py#L25 (there is an openstack common implementation for this)","commit_id":"045ad8825bfafde81a9472518dff9d49bde85883"}],"nova/virt/vmwareapi/fake.py":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"76b6102d76beee463c498c85e65407da336efce4","unresolved":false,"context_lines":[{"line_number":869,"context_line":""},{"line_number":870,"context_line":"    def _login(self):"},{"line_number":871,"context_line":"        \"\"\"Logs in and sets the session object in the db.\"\"\""},{"line_number":872,"context_line":"        self._session \u003d uuidutils.generate_uuid()"},{"line_number":873,"context_line":"        session \u003d DataObject()"},{"line_number":874,"context_line":"        session.key \u003d self._session"},{"line_number":875,"context_line":"        _db_content[\u0027session\u0027][self._session] \u003d session"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FtFY%3D","line":872,"updated":"2013-10-30 07:54:44.000000000","message":"This change seems unrelated ?","commit_id":"d299cfb8b4e609574e3bf594f706a0df6ce4bcaa"}],"nova/virt/vmwareapi/vmops.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8e3641d9bf165e8b0518e8ef33615d87142161ef","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                datacenter\u003ddc_ref)"},{"line_number":155,"context_line":"        self._session._wait_for_task(instance[\u0027uuid\u0027],"},{"line_number":156,"context_line":"                                     file_delete_task)"},{"line_number":157,"context_line":"        LOG.debug(_(\"Deleted the datastore file %s\"), instance\u003dinstance)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def spawn(self, context, instance, image_meta, injected_files,"},{"line_number":160,"context_line":"              admin_password, network_info, block_device_info\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAS3%2F%2FNtQ%3D","line":157,"updated":"2013-10-02 06:27:42.000000000","message":"Missing parameter datastore_path","commit_id":"cc53e98bb2e9ec771e9ae6bb7171151d5d33efa1"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"2df5a6d9ec32156698307b27df7d700c193cb29f","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                datacenter\u003ddc_ref)"},{"line_number":155,"context_line":"        self._session._wait_for_task(instance[\u0027uuid\u0027],"},{"line_number":156,"context_line":"                                     file_delete_task)"},{"line_number":157,"context_line":"        LOG.debug(_(\"Deleted the datastore file %s\"), instance\u003dinstance)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def spawn(self, context, instance, image_meta, injected_files,"},{"line_number":160,"context_line":"              admin_password, network_info, block_device_info\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAS3%2F%2FLwg%3D","line":157,"in_reply_to":"AAAAS3%2F%2FNtQ%3D","updated":"2013-10-02 19:03:19.000000000","message":"Done. My original intention was to remove the %s.","commit_id":"cc53e98bb2e9ec771e9ae6bb7171151d5d33efa1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"6525d7f09f4432d35fe7c3181a40ac100e7b199e","unresolved":false,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    @staticmethod"},{"line_number":146,"context_line":"    def _generate_random_vmdk_name():"},{"line_number":147,"context_line":"        return str(uuid.uuid4())"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    def _delete_datastore_file(self, instance, datastore_path, dc_ref):"},{"line_number":150,"context_line":"        LOG.debug(_(\"Deleting the datastore file %s\") % datastore_path,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAS3%2F%2FKQA%3D","line":147,"updated":"2013-10-03 07:54:21.000000000","message":"please use https://github.com/openstack/nova/blob/master/nova/openstack/common/uuidutils.py#L25 (there is an openstack common implementation for this)","commit_id":"045ad8825bfafde81a9472518dff9d49bde85883"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"3fa359b7f61fe5b245456038529492660313b7fd","unresolved":false,"context_lines":[{"line_number":437,"context_line":"                    _create_virtual_disk()"},{"line_number":438,"context_line":"                    self._delete_datastore_file(instance,"},{"line_number":439,"context_line":"                                               flat_uploaded_vmdk_path,"},{"line_number":440,"context_line":"                                               dc_ref)"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"                _fetch_image_on_esx_datastore()"},{"line_number":443,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATH%2F%2FKEA%3D","line":440,"updated":"2013-10-22 11:04:58.000000000","message":"whitespace issue, need extra space on above two lines","commit_id":"1366324e53dfa25e1e7b917e46041008ad1dcc42"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"3fa359b7f61fe5b245456038529492660313b7fd","unresolved":false,"context_lines":[{"line_number":448,"context_line":"                                       uploaded_vmdk_path)"},{"line_number":449,"context_line":"                    self._delete_datastore_file(instance,"},{"line_number":450,"context_line":"                                               sparse_uploaded_vmdk_path,"},{"line_number":451,"context_line":"                                               dc_ref)"},{"line_number":452,"context_line":"            else:"},{"line_number":453,"context_line":"                # linked clone base disk exists"},{"line_number":454,"context_line":"                if disk_type \u003d\u003d \"sparse\":"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATH%2F%2FKD8%3D","line":451,"updated":"2013-10-22 11:04:58.000000000","message":"ditto","commit_id":"1366324e53dfa25e1e7b917e46041008ad1dcc42"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"3fa359b7f61fe5b245456038529492660313b7fd","unresolved":false,"context_lines":[{"line_number":740,"context_line":"        dest_vmdk_file_path \u003d vm_util.build_datastore_path(datastore_name,"},{"line_number":741,"context_line":"                   \"vmware-tmp/%s.vmdk\" % random_name)"},{"line_number":742,"context_line":"        dest_vmdk_data_file_path \u003d vm_util.build_datastore_path(datastore_name,"},{"line_number":743,"context_line":"                   \"vmware-tmp/%s-flat.vmdk\" % random_name)"},{"line_number":744,"context_line":"        dc_ref \u003d self._get_datacenter_ref_and_name()[0]"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        def _copy_vmdk_content():"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATH%2F%2FKDU%3D","line":743,"updated":"2013-10-22 11:04:58.000000000","message":"Is the -flat file an implicitly created temporary file or something?","commit_id":"1366324e53dfa25e1e7b917e46041008ad1dcc42"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"3fa359b7f61fe5b245456038529492660313b7fd","unresolved":false,"context_lines":[{"line_number":784,"context_line":"                data_center_name\u003dself._get_datacenter_ref_and_name()[1],"},{"line_number":785,"context_line":"                datastore_name\u003ddatastore_name,"},{"line_number":786,"context_line":"                cookies\u003dcookies,"},{"line_number":787,"context_line":"                file_path\u003d\"vmware-tmp/%s-flat.vmdk\" % random_name)"},{"line_number":788,"context_line":"            LOG.debug(_(\"Uploaded image %s\") % image_id,"},{"line_number":789,"context_line":"                      instance\u003dinstance)"},{"line_number":790,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATH%2F%2FKDM%3D","line":787,"updated":"2013-10-22 11:04:58.000000000","message":"It doesn\u0027t make sense to use dest_vmdk_data_file_path here?","commit_id":"1366324e53dfa25e1e7b917e46041008ad1dcc42"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"094fda88f9e8ed48b7aa50defe2b6146fcac650f","unresolved":false,"context_lines":[{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    @staticmethod"},{"line_number":147,"context_line":"    def _generate_random_vmdk_name():"},{"line_number":148,"context_line":"        return uuidutils.generate_uuid()"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    def _delete_datastore_file(self, instance, datastore_path, dc_ref):"},{"line_number":151,"context_line":"        LOG.debug(_(\"Deleting the datastore file %s\") % datastore_path,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2F%2FLw%3D","line":148,"updated":"2013-10-24 07:57:53.000000000","message":"I can see that this method makes mocking for testing easier, but why wouldn\u0027t you just mock uuidutils instead?","commit_id":"6350b2f94b2247e6fcc5dfe958c3adfa70a2ca10"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37b018d5022999a7788663503cdf363eb33ed4d7","unresolved":false,"context_lines":[{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    @staticmethod"},{"line_number":147,"context_line":"    def _generate_random_vmdk_name():"},{"line_number":148,"context_line":"        return uuidutils.generate_uuid()"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    def _delete_datastore_file(self, instance, datastore_path, dc_ref):"},{"line_number":151,"context_line":"        LOG.debug(_(\"Deleting the datastore file %s\") % datastore_path,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2F8l8%3D","line":148,"in_reply_to":"AAAATn%2F%2F%2FLw%3D","updated":"2013-10-24 19:00:14.000000000","message":"Heh, yeah, agree.","commit_id":"6350b2f94b2247e6fcc5dfe958c3adfa70a2ca10"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"76b6102d76beee463c498c85e65407da336efce4","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        # Generate a random vmdk file name to which the coalesced vmdk content"},{"line_number":742,"context_line":"        # will be copied to. A random name is chosen so that we don\u0027t have"},{"line_number":743,"context_line":"        # name clashes."},{"line_number":744,"context_line":"        random_name \u003d uuidutils.generate_uuid()"},{"line_number":745,"context_line":"        dest_vmdk_file_path \u003d vm_util.build_datastore_path(datastore_name,"},{"line_number":746,"context_line":"                   \"vmware-tmp/%s.vmdk\" % random_name)"},{"line_number":747,"context_line":"        dest_vmdk_data_file_path \u003d vm_util.build_datastore_path(datastore_name,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FtFE%3D","line":744,"updated":"2013-10-30 07:54:44.000000000","message":"This seems like an unrelated change; please keep to one conceptual change per changeset.","commit_id":"d299cfb8b4e609574e3bf594f706a0df6ce4bcaa"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"055022b7d3f2a8a95e4b46ee6e952c169513f6e4","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        # Generate a random vmdk file name to which the coalesced vmdk content"},{"line_number":742,"context_line":"        # will be copied to. A random name is chosen so that we don\u0027t have"},{"line_number":743,"context_line":"        # name clashes."},{"line_number":744,"context_line":"        random_name \u003d uuidutils.generate_uuid()"},{"line_number":745,"context_line":"        dest_vmdk_file_path \u003d vm_util.build_datastore_path(datastore_name,"},{"line_number":746,"context_line":"                   \"vmware-tmp/%s.vmdk\" % random_name)"},{"line_number":747,"context_line":"        dest_vmdk_data_file_path \u003d vm_util.build_datastore_path(datastore_name,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2Fii8%3D","line":744,"in_reply_to":"AAAATn%2F%2FilY%3D","updated":"2013-11-01 18:07:51.000000000","message":"Sorry I am late to the discussion :). Thanks for the comments. It\u0027s helpful to learn from both sides of the argument. FWIW, the uuid cleanup was added to address a suggestion in a review comment. I have no objection to putting that off to a separate patch. Let me clean that up, as well as respond to the file leak issue in a patch update. Thanks.","commit_id":"d299cfb8b4e609574e3bf594f706a0df6ce4bcaa"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"375d0f66374e2696d6cbbb844b2cb714f0edb781","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        # Generate a random vmdk file name to which the coalesced vmdk content"},{"line_number":742,"context_line":"        # will be copied to. A random name is chosen so that we don\u0027t have"},{"line_number":743,"context_line":"        # name clashes."},{"line_number":744,"context_line":"        random_name \u003d uuidutils.generate_uuid()"},{"line_number":745,"context_line":"        dest_vmdk_file_path \u003d vm_util.build_datastore_path(datastore_name,"},{"line_number":746,"context_line":"                   \"vmware-tmp/%s.vmdk\" % random_name)"},{"line_number":747,"context_line":"        dest_vmdk_data_file_path \u003d vm_util.build_datastore_path(datastore_name,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FilY%3D","line":744,"in_reply_to":"AAAATn%2F%2Fj%2Bg%3D","updated":"2013-11-01 17:52:57.000000000","message":"Right, I wouldn\u0027t have -1\u0027d for such a small conflation: I -1\u0027d for the possible problem with leaking files that should be deleted in the loop. I would have put a 0 comment on the conflated change.\n\nPart of this in my head at least is getting contributors to be in the habit of doing cleanups as separate commits: it\u0027s very easy to keep changes separate in the first place once you\u0027ve learnt how things can be split up - but that learning takes place in response to reviews...","commit_id":"d299cfb8b4e609574e3bf594f706a0df6ce4bcaa"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"c4f7289a53d4263cef1a864aba144f998b15d8f2","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        # Generate a random vmdk file name to which the coalesced vmdk content"},{"line_number":742,"context_line":"        # will be copied to. A random name is chosen so that we don\u0027t have"},{"line_number":743,"context_line":"        # name clashes."},{"line_number":744,"context_line":"        random_name \u003d uuidutils.generate_uuid()"},{"line_number":745,"context_line":"        dest_vmdk_file_path \u003d vm_util.build_datastore_path(datastore_name,"},{"line_number":746,"context_line":"                   \"vmware-tmp/%s.vmdk\" % random_name)"},{"line_number":747,"context_line":"        dest_vmdk_data_file_path \u003d vm_util.build_datastore_path(datastore_name,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2Fj%2Bg%3D","line":744,"in_reply_to":"AAAATn%2F%2FkMY%3D","updated":"2013-11-01 09:53:02.000000000","message":"True, but uuidutils.generate_uuid() is generally favoured across Nova over calling uuid4() directly.\n\nI\u0027m a big proponent of splitting large patches into small series of patches and, yes, you\u0027re technically correct - I just think it\u0027s overly pedantic in this case. It\u0027s pretty close to being a no-op change and it is closely related to the patch.\n\nNow, if you want to debate whether generate_uuid() should exist at all and don\u0027t want to block this patch with a debate around that, that\u0027s an entirely different thing - just submit a separate patch yourself to remove it. I don\u0027t have a strong opinion on it.\n\n(Then again, if I was the patch author and I was re-spinning the patch anyway - I probably would just roll my eyes at all of this and remove the change :)","commit_id":"d299cfb8b4e609574e3bf594f706a0df6ce4bcaa"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"d4220d8876a5df6c8e5c490c727971f8c8bb119d","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        # Generate a random vmdk file name to which the coalesced vmdk content"},{"line_number":742,"context_line":"        # will be copied to. A random name is chosen so that we don\u0027t have"},{"line_number":743,"context_line":"        # name clashes."},{"line_number":744,"context_line":"        random_name \u003d uuidutils.generate_uuid()"},{"line_number":745,"context_line":"        dest_vmdk_file_path \u003d vm_util.build_datastore_path(datastore_name,"},{"line_number":746,"context_line":"                   \"vmware-tmp/%s.vmdk\" % random_name)"},{"line_number":747,"context_line":"        dest_vmdk_data_file_path \u003d vm_util.build_datastore_path(datastore_name,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FkMY%3D","line":744,"in_reply_to":"AAAATn%2F%2FkVQ%3D","updated":"2013-11-01 08:19:51.000000000","message":"uuid4() can be mocked out directly - there is no need at all to change the function called to let the test be written.","commit_id":"d299cfb8b4e609574e3bf594f706a0df6ce4bcaa"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"795a56524d7ac2367c3cb603744d4ebcb1c9b5c9","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        # Generate a random vmdk file name to which the coalesced vmdk content"},{"line_number":742,"context_line":"        # will be copied to. A random name is chosen so that we don\u0027t have"},{"line_number":743,"context_line":"        # name clashes."},{"line_number":744,"context_line":"        random_name \u003d uuidutils.generate_uuid()"},{"line_number":745,"context_line":"        dest_vmdk_file_path \u003d vm_util.build_datastore_path(datastore_name,"},{"line_number":746,"context_line":"                   \"vmware-tmp/%s.vmdk\" % random_name)"},{"line_number":747,"context_line":"        dest_vmdk_data_file_path \u003d vm_util.build_datastore_path(datastore_name,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FkVQ%3D","line":744,"in_reply_to":"AAAATn%2F%2FtFE%3D","updated":"2013-11-01 06:46:28.000000000","message":"Strictly speaking, maybe - but you can see this no-op change was made so that it could be mocked out in a test added by this commit. That\u0027s related enough for me.","commit_id":"d299cfb8b4e609574e3bf594f706a0df6ce4bcaa"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"76b6102d76beee463c498c85e65407da336efce4","unresolved":false,"context_lines":[{"line_number":806,"context_line":"            LOG.debug(_(\"Deleting temporary vmdk file %s\")"},{"line_number":807,"context_line":"                        % dest_vmdk_file_path, instance\u003dinstance)"},{"line_number":808,"context_line":"            for f in dest_vmdk_file_path, dest_vmdk_data_file_path:"},{"line_number":809,"context_line":"                self._delete_datastore_file(instance, f, dc_ref)"},{"line_number":810,"context_line":"            LOG.debug(_(\"Deleted temporary vmdk file %s\")"},{"line_number":811,"context_line":"                        % dest_vmdk_file_path, instance\u003dinstance)"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FtCA%3D","line":809,"updated":"2013-10-30 07:54:44.000000000","message":"If this throws, only some of the files will be deleted. Should you ensure that you try to delete them all?","commit_id":"d299cfb8b4e609574e3bf594f706a0df6ce4bcaa"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"795a56524d7ac2367c3cb603744d4ebcb1c9b5c9","unresolved":false,"context_lines":[{"line_number":806,"context_line":"            LOG.debug(_(\"Deleting temporary vmdk file %s\")"},{"line_number":807,"context_line":"                        % dest_vmdk_file_path, instance\u003dinstance)"},{"line_number":808,"context_line":"            for f in dest_vmdk_file_path, dest_vmdk_data_file_path:"},{"line_number":809,"context_line":"                self._delete_datastore_file(instance, f, dc_ref)"},{"line_number":810,"context_line":"            LOG.debug(_(\"Deleted temporary vmdk file %s\")"},{"line_number":811,"context_line":"                        % dest_vmdk_file_path, instance\u003dinstance)"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FkVI%3D","line":809,"in_reply_to":"AAAATn%2F%2FtCA%3D","updated":"2013-11-01 06:46:28.000000000","message":"That\u0027s a good point","commit_id":"d299cfb8b4e609574e3bf594f706a0df6ce4bcaa"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"c4aeec4145bbf31b3253e5d43a7970f48043f273","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        # Generate a random vmdk file name to which the coalesced vmdk content"},{"line_number":742,"context_line":"        # will be copied to. A random name is chosen so that we don\u0027t have"},{"line_number":743,"context_line":"        # name clashes."},{"line_number":744,"context_line":"        random_name \u003d uuidutils.generate_uuid()"},{"line_number":745,"context_line":"        dest_vmdk_file_path \u003d vm_util.build_datastore_path(datastore_name,"},{"line_number":746,"context_line":"                   \"vmware-tmp/%s.vmdk\" % random_name)"},{"line_number":747,"context_line":"        dest_vmdk_data_file_path \u003d vm_util.build_datastore_path(datastore_name,"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F%2FiL8%3D","line":744,"updated":"2013-11-01 20:34:38.000000000","message":"@lifeless: I am proposing that we retain the use of uuidutils here after all. A case for it is that the vmwareapi unit tests, rightly or not, already switched to using uuidutils. Using uuid.uuid4() here means having to import both uuidutils and uuid for mockup purposes, which looks weird and opens its own can of worms. Hope that\u0027s okay.","commit_id":"8da25d57a61771272c7670d21368f1611024a876"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"1be599331a71b8078282f646764858438d6c5bba","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        # Generate a random vmdk file name to which the coalesced vmdk content"},{"line_number":742,"context_line":"        # will be copied to. A random name is chosen so that we don\u0027t have"},{"line_number":743,"context_line":"        # name clashes."},{"line_number":744,"context_line":"        random_name \u003d uuidutils.generate_uuid()"},{"line_number":745,"context_line":"        dest_vmdk_file_path \u003d vm_util.build_datastore_path(datastore_name,"},{"line_number":746,"context_line":"                   \"vmware-tmp/%s.vmdk\" % random_name)"},{"line_number":747,"context_line":"        dest_vmdk_data_file_path \u003d vm_util.build_datastore_path(datastore_name,"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F%2Fhuc%3D","line":744,"in_reply_to":"AAAATn%2F%2FiL8%3D","updated":"2013-11-02 06:10:26.000000000","message":"It\u0027s ok; like I said, it\u0027s not really an issue here, I mentioned it because it is a separate change, and when it\u0027s easy to do that really is better. I won\u0027t -1 this patch because of it :)","commit_id":"8da25d57a61771272c7670d21368f1611024a876"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"c4aeec4145bbf31b3253e5d43a7970f48043f273","unresolved":false,"context_lines":[{"line_number":802,"context_line":"            Delete temporary vmdk files generated in image handling"},{"line_number":803,"context_line":"            operations."},{"line_number":804,"context_line":"            \"\"\""},{"line_number":805,"context_line":"            # The data file is the one occupying space, and likelier to see"},{"line_number":806,"context_line":"            # deletion problems, so prioritize its deletion first. In the"},{"line_number":807,"context_line":"            # unlikely event that its deletion fails, the small descriptor file"},{"line_number":808,"context_line":"            # is retained too by design since it makes little sense to remove"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F%2FiMc%3D","line":805,"updated":"2013-11-01 20:34:38.000000000","message":"@lifeless,Mark: The behavior now is that we short-circuit the deletion sequence on any failure and return an exception. There is some general retry logic when we issue requests to the server to handle temporary errors that will help, but even so there is always a possibility that the deletion will fail for some odd reason. We have no recovery mechanism when the first file is deleted but we fail to delete the second, but that\u0027s something we have to live with. I did make a change to reverse the order of the files that we delete, however. First the data file is the big one, will likely see a higher probability of failing to delete too, and is the one we care to remove more, so we prioritize to delete it first. Additionally when we fail to delete it, it is a better to retain the descriptor file too, since certain APIs that can be used to examine/remove the disk in the future will require the descriptor file. Thanks.","commit_id":"8da25d57a61771272c7670d21368f1611024a876"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"1be599331a71b8078282f646764858438d6c5bba","unresolved":false,"context_lines":[{"line_number":802,"context_line":"            Delete temporary vmdk files generated in image handling"},{"line_number":803,"context_line":"            operations."},{"line_number":804,"context_line":"            \"\"\""},{"line_number":805,"context_line":"            # The data file is the one occupying space, and likelier to see"},{"line_number":806,"context_line":"            # deletion problems, so prioritize its deletion first. In the"},{"line_number":807,"context_line":"            # unlikely event that its deletion fails, the small descriptor file"},{"line_number":808,"context_line":"            # is retained too by design since it makes little sense to remove"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F%2FhuU%3D","line":805,"in_reply_to":"AAAATn%2F%2FiMc%3D","updated":"2013-11-02 06:10:26.000000000","message":"Thank you very much for adding a rationale for this.","commit_id":"8da25d57a61771272c7670d21368f1611024a876"}]}