)]}'
{"nova/tests/virt/vmwareapi/test_driver_api.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9f0ad8fc18ab7e20e682e505e53b48693e859de3","unresolved":false,"context_lines":[{"line_number":552,"context_line":"        self.stubs.Set(self.conn._vmops, \u0027_create_config_drive\u0027,"},{"line_number":553,"context_line":"                       fake_create_config_drive)"},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        self._create_vm()"},{"line_number":556,"context_line":""},{"line_number":557,"context_line":"    def test_spawn(self):"},{"line_number":558,"context_line":"        self._create_vm()"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2FrzA%3D","line":555,"updated":"2014-03-04 17:33:53.000000000","message":"I so hate these massive unit tests, its next to impossible to check if you have covered all the code.\n\nAnyways, thats just something to look at when you think about how to refactor things.","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5f66d95fef0df29230797114a70ca4ef2d0fc26a","unresolved":false,"context_lines":[{"line_number":552,"context_line":"        self.stubs.Set(self.conn._vmops, \u0027_create_config_drive\u0027,"},{"line_number":553,"context_line":"                       fake_create_config_drive)"},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        self._create_vm()"},{"line_number":556,"context_line":""},{"line_number":557,"context_line":"    def test_spawn(self):"},{"line_number":558,"context_line":"        self._create_vm()"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2FrMU%3D","line":555,"in_reply_to":"AAAAVn%2F%2FrfQ%3D","updated":"2014-03-04 18:24:58.000000000","message":"Agree with you guys. This is also part of our plans.","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"11a287a5a27693795e361d531b80a63f8660e918","unresolved":false,"context_lines":[{"line_number":552,"context_line":"        self.stubs.Set(self.conn._vmops, \u0027_create_config_drive\u0027,"},{"line_number":553,"context_line":"                       fake_create_config_drive)"},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        self._create_vm()"},{"line_number":556,"context_line":""},{"line_number":557,"context_line":"    def test_spawn(self):"},{"line_number":558,"context_line":"        self._create_vm()"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2FrfQ%3D","line":555,"in_reply_to":"AAAAVn%2F%2FrzA%3D","updated":"2014-03-04 17:58:37.000000000","message":"Further, using stubs like this and no solid mocking, these tests provide no indication that they would even fail if the paths they assume are being taken, don\u0027t get taken. IMHO, the above is not sufficient to demonstrate to me that the new paths are being taken and tested thoroughly, without me walking the 500LOC _spawn function by hand.","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"}],"nova/tests/virt/vmwareapi/test_vmwareapi.py":[{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"30ec24490db0cbdd3c93a2c408a9696ef7c1b818","unresolved":false,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        self.image \u003d {"},{"line_number":179,"context_line":"            \u0027id\u0027: \u0027c1c8ce3d-c2e0-4247-890c-ccf5cc1c004c\u0027,"},{"line_number":180,"context_line":"            \u0027disk_format\u0027: \u0027vmdk\u0027,"},{"line_number":181,"context_line":"            \u0027size\u0027: 512,"},{"line_number":182,"context_line":"        }"},{"line_number":183,"context_line":"        nova.tests.image.fake.stub_out_image_service(self.stubs)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2FvvU%3D","line":180,"updated":"2014-01-08 23:01:19.000000000","message":"huh. This doesn\u0027t invalidate anything that was testing for vhd (whatever that was)?","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"30405fc847af87db9b93f603be3ab5f742934276","unresolved":false,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        self.image \u003d {"},{"line_number":179,"context_line":"            \u0027id\u0027: \u0027c1c8ce3d-c2e0-4247-890c-ccf5cc1c004c\u0027,"},{"line_number":180,"context_line":"            \u0027disk_format\u0027: \u0027vmdk\u0027,"},{"line_number":181,"context_line":"            \u0027size\u0027: 512,"},{"line_number":182,"context_line":"        }"},{"line_number":183,"context_line":"        nova.tests.image.fake.stub_out_image_service(self.stubs)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2FuCc%3D","line":180,"in_reply_to":"AAAAUH%2F%2FvvU%3D","updated":"2014-01-09 08:40:35.000000000","message":"The disk format was actually never checked. Hence the change.","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"}],"nova/virt/vmwareapi/vmops.py":[{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"30ec24490db0cbdd3c93a2c408a9696ef7c1b818","unresolved":false,"context_lines":[{"line_number":208,"context_line":"            file_type \u003d \u0027iso\u0027"},{"line_number":209,"context_line":"        else:"},{"line_number":210,"context_line":"            file_type \u003d \u0027vmdk\u0027"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"},{"line_number":213,"context_line":"        service_content \u003d self._session._get_vim().get_service_content()"},{"line_number":214,"context_line":"        ds \u003d vm_util.get_datastore_ref_and_name(self._session, self._cluster,"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2Fvy0%3D","line":211,"updated":"2014-01-08 23:01:19.000000000","message":"Consider:\n\n    file_type \u003d \u0027iso\u0027 if image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027 else \u0027vmdk\u0027\n\n... but that\u0027s just my personal preference because it avoids cluttering the namespace with the extra \u0027iso\u0027 variable which serves no purpose outside these 5 lines.\n\nI suppose I dislike \u0027if\u0027 statements.\n\nThis particular method is *so* long I might like it if you broke this away into its own method even though that\u0027s almost a silly thing to do for these 4 lines.","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"30405fc847af87db9b93f603be3ab5f742934276","unresolved":false,"context_lines":[{"line_number":208,"context_line":"            file_type \u003d \u0027iso\u0027"},{"line_number":209,"context_line":"        else:"},{"line_number":210,"context_line":"            file_type \u003d \u0027vmdk\u0027"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"},{"line_number":213,"context_line":"        service_content \u003d self._session._get_vim().get_service_content()"},{"line_number":214,"context_line":"        ds \u003d vm_util.get_datastore_ref_and_name(self._session, self._cluster,"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2FuBg%3D","line":211,"in_reply_to":"AAAAUH%2F%2Fvy0%3D","updated":"2014-01-09 08:40:35.000000000","message":"instead of doing string comparisons throughout the code we have a boolean variable iso that will be used.\nfor the file_type i have removed the if and done as you have suggested","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"30ec24490db0cbdd3c93a2c408a9696ef7c1b818","unresolved":false,"context_lines":[{"line_number":454,"context_line":""},{"line_number":455,"context_line":"            # The vmdk meta-data file"},{"line_number":456,"context_line":"            uploaded_vmdk_name \u003d \"%s/%s.%s\" % (upload_folder, upload_name,"},{"line_number":457,"context_line":"                                               file_type)"},{"line_number":458,"context_line":"            uploaded_vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":459,"context_line":"                                                uploaded_vmdk_name)"},{"line_number":460,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2Fvyc%3D","line":457,"updated":"2014-01-08 23:01:19.000000000","message":"nice.","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"30ec24490db0cbdd3c93a2c408a9696ef7c1b818","unresolved":false,"context_lines":[{"line_number":461,"context_line":"            session_vim \u003d self._session._get_vim()"},{"line_number":462,"context_line":"            cookies \u003d session_vim.client.options.transport.cookiejar"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"            upload_file_name \u003d upload_name + \".%s\" % file_type"},{"line_number":465,"context_line":"            if not (self._check_if_folder_file_exists("},{"line_number":466,"context_line":"                                        data_store_ref, data_store_name,"},{"line_number":467,"context_line":"                                        upload_folder, upload_file_name)):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2FvyI%3D","line":464,"updated":"2014-01-08 23:01:19.000000000","message":"very nice.","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"30ec24490db0cbdd3c93a2c408a9696ef7c1b818","unresolved":false,"context_lines":[{"line_number":490,"context_line":"                                          file_type)"},{"line_number":491,"context_line":"                vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":492,"context_line":"                                                         vmdk_name)"},{"line_number":493,"context_line":"                if iso:"},{"line_number":494,"context_line":"                    upload_file_name \u003d vmdk_name"},{"line_number":495,"context_line":"                else:"},{"line_number":496,"context_line":"                    if disk_type !\u003d \"sparse\":"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2Fvxc%3D","line":493,"updated":"2014-01-08 23:01:19.000000000","message":"oh... here it is... I remember this from our days back on line 211... when was that? That was 282 lines ago... I\u0027d forgotten you old chum! If I had seen:\n\n    if file_type \u003d\u003d \u0027iso\u0027:\n\n... here instead I wouldn\u0027t have had this rush of nostalgia. Is that a good thing? That seems like a bad thing to have to hold context like that for hundreds of lines.","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"30405fc847af87db9b93f603be3ab5f742934276","unresolved":false,"context_lines":[{"line_number":490,"context_line":"                                          file_type)"},{"line_number":491,"context_line":"                vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":492,"context_line":"                                                         vmdk_name)"},{"line_number":493,"context_line":"                if iso:"},{"line_number":494,"context_line":"                    upload_file_name \u003d vmdk_name"},{"line_number":495,"context_line":"                else:"},{"line_number":496,"context_line":"                    if disk_type !\u003d \"sparse\":"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2FuBI%3D","line":493,"in_reply_to":"AAAAUH%2F%2Fvxc%3D","updated":"2014-01-09 08:40:35.000000000","message":"my take here is that there are 2 local variable:\nfile_type - \u0027vmdk\u0027 and \u0027iso\u0027 and iso - a boolean.\nnot sure i understand the point about contest. \nyes, it is sad that the methouds are terribly big but this is something that we are planning on dealing with in the future","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"30ec24490db0cbdd3c93a2c408a9696ef7c1b818","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                if iso:"},{"line_number":494,"context_line":"                    upload_file_name \u003d vmdk_name"},{"line_number":495,"context_line":"                else:"},{"line_number":496,"context_line":"                    if disk_type !\u003d \"sparse\":"},{"line_number":497,"context_line":"                        # Create a flat virtual disk and retain the metadata"},{"line_number":498,"context_line":"                        # file."},{"line_number":499,"context_line":"                        _create_virtual_disk(upload_folder, vmdk_path)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2Fvw8%3D","line":496,"updated":"2014-01-08 23:01:19.000000000","message":"notice here we\u0027re doing something with disk_type ... maybe it would have been better to have a boolean variable called \u0027sparse\u0027 up around line 211?\n\nI suppose if I were to argue *for* using a string compare it would be because that is consistent with what\u0027s here ... not that I particularly liked the idea.","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"30405fc847af87db9b93f603be3ab5f742934276","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                if iso:"},{"line_number":494,"context_line":"                    upload_file_name \u003d vmdk_name"},{"line_number":495,"context_line":"                else:"},{"line_number":496,"context_line":"                    if disk_type !\u003d \"sparse\":"},{"line_number":497,"context_line":"                        # Create a flat virtual disk and retain the metadata"},{"line_number":498,"context_line":"                        # file."},{"line_number":499,"context_line":"                        _create_virtual_disk(upload_folder, vmdk_path)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2FuBA%3D","line":496,"in_reply_to":"AAAAUH%2F%2Fvw8%3D","updated":"2014-01-09 08:40:35.000000000","message":"that is a good idea but not related to these changes.","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"451743e65a53bb46e0c4a01c4ddf3f373f1aea44","unresolved":false,"context_lines":[{"line_number":525,"context_line":"                if disk_type \u003d\u003d \"sparse\":"},{"line_number":526,"context_line":"                    disk_type \u003d \"thin\""},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"            if iso:"},{"line_number":529,"context_line":"                dest_folder \u003d instance[\u0027uuid\u0027]"},{"line_number":530,"context_line":"                dest_name \u003d instance[\u0027name\u0027]"},{"line_number":531,"context_line":"                dest_vmdk_name \u003d \"%s/%s.vmdk\" % (dest_folder,"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2Fvt0%3D","line":528,"updated":"2014-01-08 23:05:29.000000000","message":"I really don\u0027t like how messy lines 528 through 598 are getting. This particular patch pushes in quite a bit more complexity on these lines. That\u0027s even more context to track. This is pretty bad already. Do I hang this problem on this patch though? Do I let it slide again? I\u0027m on the fence.","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"30405fc847af87db9b93f603be3ab5f742934276","unresolved":false,"context_lines":[{"line_number":525,"context_line":"                if disk_type \u003d\u003d \"sparse\":"},{"line_number":526,"context_line":"                    disk_type \u003d \"thin\""},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"            if iso:"},{"line_number":529,"context_line":"                dest_folder \u003d instance[\u0027uuid\u0027]"},{"line_number":530,"context_line":"                dest_name \u003d instance[\u0027name\u0027]"},{"line_number":531,"context_line":"                dest_vmdk_name \u003d \"%s/%s.vmdk\" % (dest_folder,"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2FuAs%3D","line":528,"in_reply_to":"AAAAUH%2F%2Fvt0%3D","updated":"2014-01-09 08:40:35.000000000","message":"there are 2 flows here:\niso and vmdk","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"30ec24490db0cbdd3c93a2c408a9696ef7c1b818","unresolved":false,"context_lines":[{"line_number":610,"context_line":"                    data_store_ref,"},{"line_number":611,"context_line":"                    uploaded_vmdk_path,"},{"line_number":612,"context_line":"                    cdrom_unit_no)"},{"line_number":613,"context_line":"                cdrom_unit_no +\u003d 1"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"            if configdrive.required_by(instance):"},{"line_number":616,"context_line":"                uploaded_iso_path \u003d self._create_config_drive(instance,"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2Fvws%3D","line":613,"updated":"2014-01-08 23:01:19.000000000","message":"these above lines, I happen to like.","commit_id":"d7a423f555ab5105f1b6ef9fddf8ee81687f4fbf"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1b1000fc2a69c2b1f66f2d19d53787d60548ec08","unresolved":false,"context_lines":[{"line_number":201,"context_line":"                ebs_root \u003d True"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"        # Get the file_type from the disk format"},{"line_number":204,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":205,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAUX%2F%2B9ks%3D","line":204,"updated":"2014-02-18 00:07:22.000000000","message":"This introduces a new image-metadata-based API to the end user, which I think we should avoid until we have the list standardized and at least the interface proposed to clean up the mess we currently have.","commit_id":"f31fcc26518493ee5a674d1dc47795e1a2bc599e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"094b5dce515ba2023f69539d31189d514713499c","unresolved":false,"context_lines":[{"line_number":201,"context_line":"                ebs_root \u003d True"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"        # Get the file_type from the disk format"},{"line_number":204,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":205,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAUX%2F%2B4MU%3D","line":204,"in_reply_to":"AAAAUX%2F%2B9ks%3D","updated":"2014-02-18 13:24:45.000000000","message":"This is an existing API - please see that it is used in libvirt - https://github.com/openstack/nova/blame/master/nova/virt/libvirt/blockinfo.py#L405\nI am not sure how we can proceed from here. Please clarify.","commit_id":"f31fcc26518493ee5a674d1dc47795e1a2bc599e"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"14b317e02192a58ec70fcd4416be10fae1d5cc6d","unresolved":false,"context_lines":[{"line_number":456,"context_line":"            uploaded_vmdk_name \u003d \"%s/%s.%s\" % (upload_folder, upload_name,"},{"line_number":457,"context_line":"                                               file_type)"},{"line_number":458,"context_line":"            uploaded_vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":459,"context_line":"                                                uploaded_vmdk_name)"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"            session_vim \u003d self._session._get_vim()"},{"line_number":462,"context_line":"            cookies \u003d session_vim.client.options.transport.cookiejar"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAU3%2F%2Fe4E%3D","line":459,"updated":"2014-02-26 11:22:22.000000000","message":"Since \u0027file_type\u0027 can be \u0027iso\u0027 now, these two variables probably shouldn\u0027t have \u0027vmdk\u0027 in their names since it is misleading.","commit_id":"3888ddeb8a8cec921345ec227c2f472b93805078"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"c03974dfbb85dd1c25cb4003851ff26750eb3063","unresolved":false,"context_lines":[{"line_number":456,"context_line":"            uploaded_vmdk_name \u003d \"%s/%s.%s\" % (upload_folder, upload_name,"},{"line_number":457,"context_line":"                                               file_type)"},{"line_number":458,"context_line":"            uploaded_vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":459,"context_line":"                                                uploaded_vmdk_name)"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"            session_vim \u003d self._session._get_vim()"},{"line_number":462,"context_line":"            cookies \u003d session_vim.client.options.transport.cookiejar"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAU3%2F%2FegQ%3D","line":459,"in_reply_to":"AAAAU3%2F%2Fe4E%3D","updated":"2014-02-26 12:06:59.000000000","message":"Done","commit_id":"3888ddeb8a8cec921345ec227c2f472b93805078"},{"author":{"_account_id":7400,"name":"Tracy Jones","email":"tjones@vmware.com","username":"tracyajones"},"change_message_id":"ec49694aaeb18b6d546fecb96e9de958e31eb2e4","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        # Get the file_type from the disk format"},{"line_number":215,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":216,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"},{"line_number":219,"context_line":"        service_content \u003d self._session._get_vim().get_service_content()"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVX%2F%2Fs34%3D","line":216,"updated":"2014-02-28 22:42:24.000000000","message":"shouldn\u0027t we use constants instead of hard-coded values like this?","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"3cd39eea4add23df149845524957a185438303c3","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        # Get the file_type from the disk format"},{"line_number":215,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":216,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"},{"line_number":219,"context_line":"        service_content \u003d self._session._get_vim().get_service_content()"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVn%2F%2F81E%3D","line":216,"in_reply_to":"AAAAVX%2F%2Fs34%3D","updated":"2014-03-03 08:32:50.000000000","message":"it is something worth considering. i would prefer doing it in a separate patch where we clean up the treatment of file_types, disk_type and image_types. I think that is more of a task that we should be doing in Juno.","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"083aa4061aa946b1cf02d8242fb2dd7f1df9a651","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        # Get the file_type from the disk format"},{"line_number":215,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":216,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"},{"line_number":219,"context_line":"        service_content \u003d self._session._get_vim().get_service_content()"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVn%2F%2FvpY%3D","line":216,"in_reply_to":"AAAAVn%2F%2F81E%3D","updated":"2014-03-04 10:57:13.000000000","message":"At the same time, I think putting some of this into vm_utils or other function will make this much easier to read. But yes, Juno time for that now.","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"713166f64fdf76fb5826988384c423299136ea14","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                                          file_type)"},{"line_number":513,"context_line":"                vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":514,"context_line":"                                                         vmdk_name)"},{"line_number":515,"context_line":"                if iso:"},{"line_number":516,"context_line":"                    upload_file_name \u003d vmdk_name"},{"line_number":517,"context_line":"                else:"},{"line_number":518,"context_line":"                    if disk_type !\u003d \"sparse\":"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVX%2F%2F9BQ%3D","line":515,"updated":"2014-02-27 06:47:17.000000000","message":"nit: could remove 515-516 and move 516 after 551 as upload_file_name isn\u0027t used till then if iso.","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"3cd39eea4add23df149845524957a185438303c3","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                                          file_type)"},{"line_number":513,"context_line":"                vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":514,"context_line":"                                                         vmdk_name)"},{"line_number":515,"context_line":"                if iso:"},{"line_number":516,"context_line":"                    upload_file_name \u003d vmdk_name"},{"line_number":517,"context_line":"                else:"},{"line_number":518,"context_line":"                    if disk_type !\u003d \"sparse\":"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVn%2F%2F804%3D","line":515,"in_reply_to":"AAAAVX%2F%2F9BQ%3D","updated":"2014-03-03 08:32:50.000000000","message":"this is required as we need it on line 530 - that is, the iso will need to be fetched from glance and loaded to the datastore","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"083aa4061aa946b1cf02d8242fb2dd7f1df9a651","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                                          file_type)"},{"line_number":513,"context_line":"                vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":514,"context_line":"                                                         vmdk_name)"},{"line_number":515,"context_line":"                if iso:"},{"line_number":516,"context_line":"                    upload_file_name \u003d vmdk_name"},{"line_number":517,"context_line":"                else:"},{"line_number":518,"context_line":"                    if disk_type !\u003d \"sparse\":"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVn%2F%2Fvoo%3D","line":515,"in_reply_to":"AAAAVn%2F%2F804%3D","updated":"2014-03-04 10:57:13.000000000","message":"We need to split up these functions a bit, its insane to reason about any of this stuff at the moment.","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5c4e97dce6acbc54186e423762926742268ac22c","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                                          file_type)"},{"line_number":513,"context_line":"                vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":514,"context_line":"                                                         vmdk_name)"},{"line_number":515,"context_line":"                if iso:"},{"line_number":516,"context_line":"                    upload_file_name \u003d vmdk_name"},{"line_number":517,"context_line":"                else:"},{"line_number":518,"context_line":"                    if disk_type !\u003d \"sparse\":"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVn%2F%2FvXY%3D","line":515,"in_reply_to":"AAAAVn%2F%2Fvoo%3D","updated":"2014-03-04 11:21:08.000000000","message":"we are in the process of discussing how to write the vmops in a better and more robust fasion. i hope that we will be able to do that in the juno time framework. one of the ideas is to have handlers specific to the file and image types. that should hopefully help moving forwards. i am not sure it is relevant here","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":7400,"name":"Tracy Jones","email":"tjones@vmware.com","username":"tracyajones"},"change_message_id":"ec49694aaeb18b6d546fecb96e9de958e31eb2e4","unresolved":false,"context_lines":[{"line_number":530,"context_line":"                _fetch_image_on_datastore(upload_file_name)"},{"line_number":531,"context_line":""},{"line_number":532,"context_line":"                if not iso and disk_type \u003d\u003d \"sparse\":"},{"line_number":533,"context_line":"                    # Copy the sparse virtual disk to a thin virtual disk."},{"line_number":534,"context_line":"                    disk_type \u003d \"thin\""},{"line_number":535,"context_line":"                    _copy_virtual_disk(sparse_uploaded_vmdk_path, vmdk_path)"},{"line_number":536,"context_line":"                    self._delete_datastore_file(instance,"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVX%2F%2Fs0I%3D","line":533,"updated":"2014-02-28 22:42:24.000000000","message":"if you decide you use constants you could change this too since you are already touching this line","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"3cd39eea4add23df149845524957a185438303c3","unresolved":false,"context_lines":[{"line_number":530,"context_line":"                _fetch_image_on_datastore(upload_file_name)"},{"line_number":531,"context_line":""},{"line_number":532,"context_line":"                if not iso and disk_type \u003d\u003d \"sparse\":"},{"line_number":533,"context_line":"                    # Copy the sparse virtual disk to a thin virtual disk."},{"line_number":534,"context_line":"                    disk_type \u003d \"thin\""},{"line_number":535,"context_line":"                    _copy_virtual_disk(sparse_uploaded_vmdk_path, vmdk_path)"},{"line_number":536,"context_line":"                    self._delete_datastore_file(instance,"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVn%2F%2F80Y%3D","line":533,"in_reply_to":"AAAAVX%2F%2Fs0I%3D","updated":"2014-03-03 08:32:50.000000000","message":"please see above.","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"083aa4061aa946b1cf02d8242fb2dd7f1df9a651","unresolved":false,"context_lines":[{"line_number":573,"context_line":"                    _copy_virtual_disk(uploaded_file_path, dest_vmdk_path)"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"                    root_vmdk_path \u003d dest_vmdk_path"},{"line_number":576,"context_line":"                    if root_gb_in_kb \u003e vmdk_file_size_in_kb:"},{"line_number":577,"context_line":"                        self._extend_virtual_disk(instance, root_gb_in_kb,"},{"line_number":578,"context_line":"                                                  root_vmdk_path, dc_info.ref)"},{"line_number":579,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVn%2F%2Fvjw%3D","line":576,"updated":"2014-03-04 10:57:13.000000000","message":"Seems like a cut and paste of the iso cast. surely if iso or not linked_clone would work better here? I know copy vs create, which I can\u0027t decide if its right or not, since you probably want to create a cache of ISO disks like any other image.","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"68ad27cdccc756f0bd6478148f23a61dc2dc655b","unresolved":false,"context_lines":[{"line_number":573,"context_line":"                    _copy_virtual_disk(uploaded_file_path, dest_vmdk_path)"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"                    root_vmdk_path \u003d dest_vmdk_path"},{"line_number":576,"context_line":"                    if root_gb_in_kb \u003e vmdk_file_size_in_kb:"},{"line_number":577,"context_line":"                        self._extend_virtual_disk(instance, root_gb_in_kb,"},{"line_number":578,"context_line":"                                                  root_vmdk_path, dc_info.ref)"},{"line_number":579,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVn%2F%2FvI0%3D","line":576,"in_reply_to":"AAAAVn%2F%2FvVc%3D","updated":"2014-03-04 11:50:48.000000000","message":"i have addressed your comments","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5c4e97dce6acbc54186e423762926742268ac22c","unresolved":false,"context_lines":[{"line_number":573,"context_line":"                    _copy_virtual_disk(uploaded_file_path, dest_vmdk_path)"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"                    root_vmdk_path \u003d dest_vmdk_path"},{"line_number":576,"context_line":"                    if root_gb_in_kb \u003e vmdk_file_size_in_kb:"},{"line_number":577,"context_line":"                        self._extend_virtual_disk(instance, root_gb_in_kb,"},{"line_number":578,"context_line":"                                                  root_vmdk_path, dc_info.ref)"},{"line_number":579,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAVn%2F%2FvVc%3D","line":576,"in_reply_to":"AAAAVn%2F%2Fvjw%3D","updated":"2014-03-04 11:21:08.000000000","message":"the iso code is handled in lines 551 - 560. the size of the disk that is created is the root_gb_in_kb\nthis code here is lines 541 - 587 (on the left hand side - shifted to be part of the not iso treatment. the code has the following updates:\n1. paths are changed for the file_type support\n2. _copy_virtual_disk is used to simply the code a little","commit_id":"80f3d3aebe3d81a49329fa76cdaff1d2508d2b16"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9f0ad8fc18ab7e20e682e505e53b48693e859de3","unresolved":false,"context_lines":[{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        # Get the file_type from the disk format"},{"line_number":218,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":219,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"},{"line_number":222,"context_line":"        service_content \u003d self._session._get_vim().get_service_content()"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2Fr5s%3D","line":219,"updated":"2014-03-04 17:33:53.000000000","message":"Thinking about it, this seems a bid dodgy, should really error out if its an unsupported format, I would think.\n\nAnyway, thats not a fix for this patch really. Please raise a bug to track that.","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5f66d95fef0df29230797114a70ca4ef2d0fc26a","unresolved":false,"context_lines":[{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        # Get the file_type from the disk format"},{"line_number":218,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":219,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"},{"line_number":222,"context_line":"        service_content \u003d self._session._get_vim().get_service_content()"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2FrfI%3D","line":219,"in_reply_to":"AAAAVn%2F%2Fr5s%3D","updated":"2014-03-04 18:24:58.000000000","message":"Done","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"11a287a5a27693795e361d531b80a63f8660e918","unresolved":false,"context_lines":[{"line_number":406,"context_line":"                LOG.debug(_(\"File %(folder)s already exists on %(ds)s.\"),"},{"line_number":407,"context_line":"                          {\u0027folder\u0027: base_folder, \u0027ds\u0027: data_store_name})"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"        def _fetch_image_on_datastore(upload_name):"},{"line_number":410,"context_line":"            \"\"\"Fetch image from Glance to datastore.\"\"\""},{"line_number":411,"context_line":"            LOG.debug(_(\"Downloading image file data %(image_ref)s to the \""},{"line_number":412,"context_line":"                        \"data store %(data_store_name)s\") %"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2Frig%3D","line":409,"updated":"2014-03-04 17:58:37.000000000","message":"This _spawn method, with its hundred nested functions is really (really) hard to follow. Going from using a variable in the parent scope to a parameter like this is really difficult to verify. IMHO, this whole thing needs some massive work so that validating and testing these changes is much easier.","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"84d65c4d3325064be2c6f2df1a693ead281356ca","unresolved":false,"context_lines":[{"line_number":406,"context_line":"                LOG.debug(_(\"File %(folder)s already exists on %(ds)s.\"),"},{"line_number":407,"context_line":"                          {\u0027folder\u0027: base_folder, \u0027ds\u0027: data_store_name})"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"        def _fetch_image_on_datastore(upload_name):"},{"line_number":410,"context_line":"            \"\"\"Fetch image from Glance to datastore.\"\"\""},{"line_number":411,"context_line":"            LOG.debug(_(\"Downloading image file data %(image_ref)s to the \""},{"line_number":412,"context_line":"                        \"data store %(data_store_name)s\") %"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2FkmQ%3D","line":409,"in_reply_to":"AAAAVn%2F%2FqpI%3D","updated":"2014-03-05 09:56:24.000000000","message":"chicken and egg, here, we avoid patches we just can\u0027t understand. took me 20-30 mins to review this the first time, most patches take 5-10 mins or so. been here and chickened out a few times.","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0d3170c07350ae76bce9f4e893a14ff222350433","unresolved":false,"context_lines":[{"line_number":406,"context_line":"                LOG.debug(_(\"File %(folder)s already exists on %(ds)s.\"),"},{"line_number":407,"context_line":"                          {\u0027folder\u0027: base_folder, \u0027ds\u0027: data_store_name})"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"        def _fetch_image_on_datastore(upload_name):"},{"line_number":410,"context_line":"            \"\"\"Fetch image from Glance to datastore.\"\"\""},{"line_number":411,"context_line":"            LOG.debug(_(\"Downloading image file data %(image_ref)s to the \""},{"line_number":412,"context_line":"                        \"data store %(data_store_name)s\") %"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2FqpI%3D","line":409,"in_reply_to":"AAAAVn%2F%2Frig%3D","updated":"2014-03-04 19:14:10.000000000","message":"This is an example of a change which would have been done a while back if the review cycle was shorter.","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5f66d95fef0df29230797114a70ca4ef2d0fc26a","unresolved":false,"context_lines":[{"line_number":406,"context_line":"                LOG.debug(_(\"File %(folder)s already exists on %(ds)s.\"),"},{"line_number":407,"context_line":"                          {\u0027folder\u0027: base_folder, \u0027ds\u0027: data_store_name})"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"        def _fetch_image_on_datastore(upload_name):"},{"line_number":410,"context_line":"            \"\"\"Fetch image from Glance to datastore.\"\"\""},{"line_number":411,"context_line":"            LOG.debug(_(\"Downloading image file data %(image_ref)s to the \""},{"line_number":412,"context_line":"                        \"data store %(data_store_name)s\") %"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2FrXE%3D","line":409,"in_reply_to":"AAAAVn%2F%2Frig%3D","updated":"2014-03-04 18:24:58.000000000","message":"i agree with you. in Juno we plan to address all of these nested functions. I really hope that this will not a blocker for this patch set","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"11a287a5a27693795e361d531b80a63f8660e918","unresolved":false,"context_lines":[{"line_number":513,"context_line":"                                                    sparse_uploaded_vmdk_name)"},{"line_number":514,"context_line":""},{"line_number":515,"context_line":"                vmdk_name \u003d \"%s/%s.%s\" % (upload_folder, upload_name,"},{"line_number":516,"context_line":"                                          file_type)"},{"line_number":517,"context_line":"                vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":518,"context_line":"                                                         vmdk_name)"},{"line_number":519,"context_line":"                if iso:"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2FrlU%3D","line":516,"updated":"2014-03-04 17:58:37.000000000","message":"Isn\u0027t this just upload_file_name as created above?\n\nThere is a lot of noise in this patch around recalculating these paths. Some organization would really help I think.","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5f66d95fef0df29230797114a70ca4ef2d0fc26a","unresolved":false,"context_lines":[{"line_number":513,"context_line":"                                                    sparse_uploaded_vmdk_name)"},{"line_number":514,"context_line":""},{"line_number":515,"context_line":"                vmdk_name \u003d \"%s/%s.%s\" % (upload_folder, upload_name,"},{"line_number":516,"context_line":"                                          file_type)"},{"line_number":517,"context_line":"                vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":518,"context_line":"                                                         vmdk_name)"},{"line_number":519,"context_line":"                if iso:"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2FrQM%3D","line":516,"in_reply_to":"AAAAVn%2F%2FrlU%3D","updated":"2014-03-04 18:24:58.000000000","message":"Done\n\nAs mentioned above in Juno we are planning a cleanup of the vmops. I am in the process of drawing up a wiki with ideas to bring to the community for discussion","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"19f3d23d5908bb96cbaf0f46a02cea04c3b43acd","unresolved":false,"context_lines":[{"line_number":515,"context_line":"                vmdk_name \u003d \"%s/%s.%s\" % (upload_folder, upload_name,"},{"line_number":516,"context_line":"                                          file_type)"},{"line_number":517,"context_line":"                vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":518,"context_line":"                                                         vmdk_name)"},{"line_number":519,"context_line":"                if iso:"},{"line_number":520,"context_line":"                    # Ensure that the ISO will be fetched from glance and"},{"line_number":521,"context_line":"                    # will be cached for future reference"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2Frrs%3D","line":518,"updated":"2014-03-04 17:45:22.000000000","message":"Another misleading variable name i missed last time around. \u0027file_type\u0027 can be \u0027iso\u0027 or \u0027vmdk\u0027 so neither of these vars should have \u0027vmdk\u0027 in their name IMHO.","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5f66d95fef0df29230797114a70ca4ef2d0fc26a","unresolved":false,"context_lines":[{"line_number":515,"context_line":"                vmdk_name \u003d \"%s/%s.%s\" % (upload_folder, upload_name,"},{"line_number":516,"context_line":"                                          file_type)"},{"line_number":517,"context_line":"                vmdk_path \u003d vm_util.build_datastore_path(data_store_name,"},{"line_number":518,"context_line":"                                                         vmdk_name)"},{"line_number":519,"context_line":"                if iso:"},{"line_number":520,"context_line":"                    # Ensure that the ISO will be fetched from glance and"},{"line_number":521,"context_line":"                    # will be cached for future reference"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAVn%2F%2FrfA%3D","line":518,"in_reply_to":"AAAAVn%2F%2Frrs%3D","updated":"2014-03-04 18:24:58.000000000","message":"Done","commit_id":"f25b956559b5f7d212eb6bd4b5f897f039fd02da"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"e3781c5c38fcb79d8c741c07a162b460c674b738","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def _get_vmdk_path(self, ds_name, folder, name):"},{"line_number":183,"context_line":"        path \u003d \"%s/%s.vmdk\" % (folder, name)"},{"line_number":184,"context_line":"        return vm_util.build_datastore_path(ds_name, path)"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    def spawn(self, context, instance, image_meta, injected_files,"},{"line_number":187,"context_line":"              admin_password, network_info, block_device_info\u003dNone,"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAVn%2F%2Fj1s%3D","line":184,"updated":"2014-03-05 12:17:24.000000000","message":"This needs to be rebased because of https://review.openstack.org/#/c/63933/ which is on it\u0027s way to the gate as if this writing.","commit_id":"32b82ab2e6610dba2e6f79b733edd068970c86c1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"56536544b69186669806a49e5839dd83f55af5e4","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def _get_vmdk_path(self, ds_name, folder, name):"},{"line_number":183,"context_line":"        path \u003d \"%s/%s.vmdk\" % (folder, name)"},{"line_number":184,"context_line":"        return vm_util.build_datastore_path(ds_name, path)"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    def spawn(self, context, instance, image_meta, injected_files,"},{"line_number":187,"context_line":"              admin_password, network_info, block_device_info\u003dNone,"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAVn%2F%2Fij0%3D","line":184,"in_reply_to":"AAAAVn%2F%2Fj1s%3D","updated":"2014-03-05 14:32:06.000000000","message":"Done","commit_id":"32b82ab2e6610dba2e6f79b733edd068970c86c1"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"e3781c5c38fcb79d8c741c07a162b460c674b738","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                ebs_root \u003d True"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        # Get the file_type from the disk format"},{"line_number":218,"context_line":"        # TODO(garyk): raise an error if disk format is not supported"},{"line_number":219,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":220,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":221,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAVn%2F%2Fjxw%3D","line":218,"updated":"2014-03-05 12:17:24.000000000","message":"Why is this a TODO? It\u0027s literally a line of code?","commit_id":"32b82ab2e6610dba2e6f79b733edd068970c86c1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"56536544b69186669806a49e5839dd83f55af5e4","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                ebs_root \u003d True"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        # Get the file_type from the disk format"},{"line_number":218,"context_line":"        # TODO(garyk): raise an error if disk format is not supported"},{"line_number":219,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":220,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":221,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAVn%2F%2FijM%3D","line":218,"in_reply_to":"AAAAVn%2F%2Fjxw%3D","updated":"2014-03-05 14:32:06.000000000","message":"i did not want to add any more noise to the code - please see https://review.openstack.org/#/c/77965/","commit_id":"32b82ab2e6610dba2e6f79b733edd068970c86c1"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"e3781c5c38fcb79d8c741c07a162b460c674b738","unresolved":false,"context_lines":[{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        # Get the file_type from the disk format"},{"line_number":218,"context_line":"        # TODO(garyk): raise an error if disk format is not supported"},{"line_number":219,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":220,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAVn%2F%2FjvU%3D","line":219,"updated":"2014-03-05 12:17:24.000000000","message":"having this variable and then having a lot of if iso: is really not ideal. What happens if we add a qcow? this method is already way to unreadable and this will make it worse really. A slightly better thing would be to factor it out in different nested methods... but really we should remove those going forward","commit_id":"32b82ab2e6610dba2e6f79b733edd068970c86c1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"56536544b69186669806a49e5839dd83f55af5e4","unresolved":false,"context_lines":[{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        # Get the file_type from the disk format"},{"line_number":218,"context_line":"        # TODO(garyk): raise an error if disk format is not supported"},{"line_number":219,"context_line":"        iso \u003d image_meta.get(\u0027disk_format\u0027) \u003d\u003d \u0027iso\u0027"},{"line_number":220,"context_line":"        file_type \u003d \u0027iso\u0027 if iso else \u0027vmdk\u0027"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        client_factory \u003d self._session._get_vim().client.factory"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAVn%2F%2Fiis%3D","line":219,"in_reply_to":"AAAAVn%2F%2FjvU%3D","updated":"2014-03-05 14:32:06.000000000","message":"we are in the process of redesigning the vmops. we will certainl address all of these concerns moving forwards","commit_id":"32b82ab2e6610dba2e6f79b733edd068970c86c1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9926817e04eb4ade6a4383afeec746c99c579e73","unresolved":false,"context_lines":[{"line_number":341,"context_line":"                folder_path \u003d ds_util.build_datastore_path(data_store_name,"},{"line_number":342,"context_line":"                                                           folder)"},{"line_number":343,"context_line":"                LOG.debug(_(\"Creating temporary folder %s\"), folder_path)"},{"line_number":344,"context_line":"                ds_util.mkdir(self._session, folder_path, dc_info.ref)"},{"line_number":345,"context_line":"            LOG.debug(_(\"Creating Virtual Disk of size  \""},{"line_number":346,"context_line":"                      \"%(vmdk_file_size_in_kb)s KB and adapter type \""},{"line_number":347,"context_line":"                      \"%(adapter_type)s on the ESX host local store \""}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAVn%2F%2FXFU%3D","line":344,"updated":"2014-03-06 16:34:53.000000000","message":"This will fail if the folder already exists, right? It\u0027s really hard to tell what the \"folder\" is here, but I think it is:\n\n base_folder/image_uuid\n\nwhich becomes this for the mkdir:\n\n [ds] base_folder/image_uuid\n\nIn that case, how does this not fail if two instances are booting from the same image sharing the same datastore?","commit_id":"566b3e957593bccdc78afa09ed55eacfbe015b4a"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"cc0152cbf858daf67202f6892b65692a95e7702b","unresolved":false,"context_lines":[{"line_number":341,"context_line":"                folder_path \u003d ds_util.build_datastore_path(data_store_name,"},{"line_number":342,"context_line":"                                                           folder)"},{"line_number":343,"context_line":"                LOG.debug(_(\"Creating temporary folder %s\"), folder_path)"},{"line_number":344,"context_line":"                ds_util.mkdir(self._session, folder_path, dc_info.ref)"},{"line_number":345,"context_line":"            LOG.debug(_(\"Creating Virtual Disk of size  \""},{"line_number":346,"context_line":"                      \"%(vmdk_file_size_in_kb)s KB and adapter type \""},{"line_number":347,"context_line":"                      \"%(adapter_type)s on the ESX host local store \""}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAVn%2F%2FUsw%3D","line":344,"in_reply_to":"AAAAVn%2F%2FUzI%3D","updated":"2014-03-06 20:44:53.000000000","message":"Done","commit_id":"566b3e957593bccdc78afa09ed55eacfbe015b4a"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"453f0e6231b1e01bb893b7e936c81c9e11548cbe","unresolved":false,"context_lines":[{"line_number":341,"context_line":"                folder_path \u003d ds_util.build_datastore_path(data_store_name,"},{"line_number":342,"context_line":"                                                           folder)"},{"line_number":343,"context_line":"                LOG.debug(_(\"Creating temporary folder %s\"), folder_path)"},{"line_number":344,"context_line":"                ds_util.mkdir(self._session, folder_path, dc_info.ref)"},{"line_number":345,"context_line":"            LOG.debug(_(\"Creating Virtual Disk of size  \""},{"line_number":346,"context_line":"                      \"%(vmdk_file_size_in_kb)s KB and adapter type \""},{"line_number":347,"context_line":"                      \"%(adapter_type)s on the ESX host local store \""}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAVn%2F%2FUzI%3D","line":344,"in_reply_to":"AAAAVn%2F%2FXFU%3D","updated":"2014-03-06 20:25:57.000000000","message":"As written the folder to create is always a temporary, unique one derived from generate_uuid(). That said the behavior could be made clearer with better comments or even a method rename. \nAlternatively don\u0027t complicate the this method with the optional mkdir at all.","commit_id":"566b3e957593bccdc78afa09ed55eacfbe015b4a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"afa4bc9478e92d2e8e6e3f8ee148f27a89870286","unresolved":false,"context_lines":[{"line_number":465,"context_line":"                flat_uploaded_vmdk_name \u003d \"%s/%s-flat.vmdk\" % ("},{"line_number":466,"context_line":"                                            upload_folder, upload_name)"},{"line_number":467,"context_line":"                # The sparse vmdk file name for sparse disk image"},{"line_number":468,"context_line":"                sparse_uploaded_vmdk_name \u003d \"%s/%s-sparse.vmdk\" % ("},{"line_number":469,"context_line":"                                            upload_folder, upload_name)"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"                flat_uploaded_vmdk_path \u003d ds_util.build_datastore_path("}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAVn%2F%2FX60%3D","line":468,"updated":"2014-03-06 15:15:24.000000000","message":"This should be defined where it is used now that it is not needed for the _fetch_image_on_datastore. Makes it a bit easier to follow this.","commit_id":"566b3e957593bccdc78afa09ed55eacfbe015b4a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"afa4bc9478e92d2e8e6e3f8ee148f27a89870286","unresolved":false,"context_lines":[{"line_number":471,"context_line":"                flat_uploaded_vmdk_path \u003d ds_util.build_datastore_path("},{"line_number":472,"context_line":"                                                    data_store_name,"},{"line_number":473,"context_line":"                                                    flat_uploaded_vmdk_name)"},{"line_number":474,"context_line":"                sparse_uploaded_vmdk_path \u003d ds_util.build_datastore_path("},{"line_number":475,"context_line":"                                                    data_store_name,"},{"line_number":476,"context_line":"                                                    sparse_uploaded_vmdk_name)"},{"line_number":477,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAVn%2F%2FX5U%3D","line":474,"updated":"2014-03-06 15:15:24.000000000","message":"These ones too?","commit_id":"566b3e957593bccdc78afa09ed55eacfbe015b4a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"afa4bc9478e92d2e8e6e3f8ee148f27a89870286","unresolved":false,"context_lines":[{"line_number":494,"context_line":""},{"line_number":495,"context_line":"                _fetch_image_on_datastore(upload_file_name)"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":"                if not iso and disk_type \u003d\u003d \"sparse\":"},{"line_number":498,"context_line":"                    # Copy the sparse virtual disk to a thin virtual disk."},{"line_number":499,"context_line":"                    disk_type \u003d \"thin\""},{"line_number":500,"context_line":"                    _copy_virtual_disk(sparse_uploaded_vmdk_path, upload_path)"}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAVn%2F%2FX2Q%3D","line":497,"updated":"2014-03-06 15:15:24.000000000","message":"This branch is basically the same as the else on line 492. Why is it even here? Can\u0027t we call _fetch_image_on_datastore on every branch... this is so difficult to follow, it\u0027s almost like it was meant to be deceiving...","commit_id":"566b3e957593bccdc78afa09ed55eacfbe015b4a"},{"author":{"_account_id":7400,"name":"Tracy Jones","email":"tjones@vmware.com","username":"tracyajones"},"change_message_id":"9b82889ad3ad650346e3609885a3fdfa0f480b39","unresolved":false,"context_lines":[{"line_number":443,"context_line":"            session_vim \u003d self._session._get_vim()"},{"line_number":444,"context_line":"            cookies \u003d session_vim.client.options.transport.cookiejar"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"            upload_file_name \u003d upload_name + \".%s\" % file_type"},{"line_number":447,"context_line":"            # Check if the image exists in the datastore cache. If not the"},{"line_number":448,"context_line":"            # image will be uploaded and cached."},{"line_number":449,"context_line":"            if not (self._check_if_folder_file_exists("}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAVn%2F%2FShc%3D","line":446,"updated":"2014-03-07 01:29:35.000000000","message":"is this line needed?  You just constructed the name in 438","commit_id":"a995485ee8b6472a5c3f9152cf0c45b5a62c312c"}]}