)]}'
{"nova/tests/virt/vmwareapi/db_fakes.py":[{"author":{"_account_id":8119,"name":"Eric Brown","email":"eric_wade_brown@yahoo.com","username":"ericwb"},"change_message_id":"dfffbe1fc8841b900f16200a2fcd68dc1eab85f7","unresolved":false,"context_lines":[{"line_number":85,"context_line":"            \u0027root_gb\u0027: type_data[\u0027root_gb\u0027],"},{"line_number":86,"context_line":"            \u0027node\u0027: values[\u0027node\u0027],"},{"line_number":87,"context_line":"            \u0027metadata\u0027: [],"},{"line_number":88,"context_line":"            \u0027host\u0027: CONF.host,"},{"line_number":89,"context_line":"            }"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        return base_options"}],"source_content_type":"text/x-python","patch_set":33,"id":"AAAAUH%2F9U0U%3D","line":88,"updated":"2014-02-07 18:12:36.000000000","message":"Trailing comma not needed.","commit_id":"5bc966d57b1c56303b65e0ba824d1e86153f4705"}],"nova/tests/virt/vmwareapi/test_driver_api.py":[{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"91e1910538ab95ff83eafe172fc6bd63b48a2517","unresolved":false,"context_lines":[{"line_number":415,"context_line":"        for vm in vms.objects:"},{"line_number":416,"context_line":"            if vm.get(\u0027name\u0027) \u003d\u003d self.uuid:"},{"line_number":417,"context_line":"                break"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"        # Check that m1.large above turned into the right thing."},{"line_number":420,"context_line":"        mem_kib \u003d long(self.type_data[\u0027memory_mb\u0027]) \u003c\u003c 10"},{"line_number":421,"context_line":"        vcpus \u003d self.type_data[\u0027vcpus\u0027]"}],"source_content_type":"text/x-python","patch_set":51,"id":"AAAAVX%2F%2F88I%3D","line":418,"updated":"2014-02-27 07:04:40.000000000","message":"I guess we don\u0027t need a for else (raise error) loop here because the vm record here should always be found?","commit_id":"82fc8cf881633c179338c00874283b09705f09f4"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e34aa2765a61b8737bef5eda73f632fc285a64f0","unresolved":false,"context_lines":[{"line_number":1619,"context_line":""},{"line_number":1620,"context_line":"    def test_timestamp_file_removed_aging(self):"},{"line_number":1621,"context_line":"        self._timestamp_file_removed()"},{"line_number":1622,"context_line":"        # mark as aged by another compute node"},{"line_number":1623,"context_line":"        ts \u003d self._get_timestamp_filename()"},{"line_number":1624,"context_line":"        ts_path \u003d (\u0027[%s] vmware_base/fake_image_uuid/%s/\u0027 %"},{"line_number":1625,"context_line":"                   (self.ds, ts))"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6pk%3D","line":1622,"updated":"2014-03-11 11:08:10.000000000","message":"Nit: This will no longer happen. The test is still valid, though, because the current node could have previously aged it.","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0e1e8112f6bd210c8bd28e95444d45a3deb5531b","unresolved":false,"context_lines":[{"line_number":1619,"context_line":""},{"line_number":1620,"context_line":"    def test_timestamp_file_removed_aging(self):"},{"line_number":1621,"context_line":"        self._timestamp_file_removed()"},{"line_number":1622,"context_line":"        # mark as aged by another compute node"},{"line_number":1623,"context_line":"        ts \u003d self._get_timestamp_filename()"},{"line_number":1624,"context_line":"        ts_path \u003d (\u0027[%s] vmware_base/fake_image_uuid/%s/\u0027 %"},{"line_number":1625,"context_line":"                   (self.ds, ts))"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6Tg%3D","line":1622,"in_reply_to":"AAAAVn%2F%2B6pk%3D","updated":"2014-03-11 11:37:47.000000000","message":"Done","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e34aa2765a61b8737bef5eda73f632fc285a64f0","unresolved":false,"context_lines":[{"line_number":1623,"context_line":"        ts \u003d self._get_timestamp_filename()"},{"line_number":1624,"context_line":"        ts_path \u003d (\u0027[%s] vmware_base/fake_image_uuid/%s/\u0027 %"},{"line_number":1625,"context_line":"                   (self.ds, ts))"},{"line_number":1626,"context_line":"        vmwareapi_fake._add_file(ts_path)"},{"line_number":1627,"context_line":"        self._timestamp_file_exists()"},{"line_number":1628,"context_line":"        all_instances \u003d [self.instance]"},{"line_number":1629,"context_line":"        self.conn.manage_image_cache(self.context, all_instances)"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6pM%3D","line":1626,"updated":"2014-03-11 11:08:10.000000000","message":"Nit: I think you could have just run the aging code once.","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0e1e8112f6bd210c8bd28e95444d45a3deb5531b","unresolved":false,"context_lines":[{"line_number":1623,"context_line":"        ts \u003d self._get_timestamp_filename()"},{"line_number":1624,"context_line":"        ts_path \u003d (\u0027[%s] vmware_base/fake_image_uuid/%s/\u0027 %"},{"line_number":1625,"context_line":"                   (self.ds, ts))"},{"line_number":1626,"context_line":"        vmwareapi_fake._add_file(ts_path)"},{"line_number":1627,"context_line":"        self._timestamp_file_exists()"},{"line_number":1628,"context_line":"        all_instances \u003d [self.instance]"},{"line_number":1629,"context_line":"        self.conn.manage_image_cache(self.context, all_instances)"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6Tc%3D","line":1626,"in_reply_to":"AAAAVn%2F%2B6pM%3D","updated":"2014-03-11 11:37:47.000000000","message":"the aging here is only run once - that is on line 1629","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e34aa2765a61b8737bef5eda73f632fc285a64f0","unresolved":false,"context_lines":[{"line_number":1635,"context_line":"        self._create_vm()"},{"line_number":1636,"context_line":"        self._cached_files_exist()"},{"line_number":1637,"context_line":"        all_instances \u003d []"},{"line_number":1638,"context_line":"        self.conn.manage_image_cache(self.context, all_instances)"},{"line_number":1639,"context_line":"        self._cached_files_exist(exists\u003dTrue)"},{"line_number":1640,"context_line":""},{"line_number":1641,"context_line":"    def _image_aging_aged(self, aging_time\u003d100):"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6mo%3D","line":1638,"updated":"2014-03-11 11:08:10.000000000","message":"I think we need to call this twice for the test below to be valid.","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0e1e8112f6bd210c8bd28e95444d45a3deb5531b","unresolved":false,"context_lines":[{"line_number":1635,"context_line":"        self._create_vm()"},{"line_number":1636,"context_line":"        self._cached_files_exist()"},{"line_number":1637,"context_line":"        all_instances \u003d []"},{"line_number":1638,"context_line":"        self.conn.manage_image_cache(self.context, all_instances)"},{"line_number":1639,"context_line":"        self._cached_files_exist(exists\u003dTrue)"},{"line_number":1640,"context_line":""},{"line_number":1641,"context_line":"    def _image_aging_aged(self, aging_time\u003d100):"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6TU%3D","line":1638,"in_reply_to":"AAAAVn%2F%2B6mo%3D","updated":"2014-03-11 11:37:47.000000000","message":"Done","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e34aa2765a61b8737bef5eda73f632fc285a64f0","unresolved":false,"context_lines":[{"line_number":1640,"context_line":""},{"line_number":1641,"context_line":"    def _image_aging_aged(self, aging_time\u003d100):"},{"line_number":1642,"context_line":"        self._override_time()"},{"line_number":1643,"context_line":"        self.cur_time \u003d datetime.datetime(2012, 11, 22, 12, 00, 10)"},{"line_number":1644,"context_line":"        self.flags(remove_unused_original_minimum_age_seconds\u003daging_time)"},{"line_number":1645,"context_line":"        self._image_aging_image_marked_for_deletion()"},{"line_number":1646,"context_line":"        all_instances \u003d []"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6kE%3D","line":1643,"updated":"2014-03-11 11:08:10.000000000","message":"Nit: this could be a local.","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0e1e8112f6bd210c8bd28e95444d45a3deb5531b","unresolved":false,"context_lines":[{"line_number":1640,"context_line":""},{"line_number":1641,"context_line":"    def _image_aging_aged(self, aging_time\u003d100):"},{"line_number":1642,"context_line":"        self._override_time()"},{"line_number":1643,"context_line":"        self.cur_time \u003d datetime.datetime(2012, 11, 22, 12, 00, 10)"},{"line_number":1644,"context_line":"        self.flags(remove_unused_original_minimum_age_seconds\u003daging_time)"},{"line_number":1645,"context_line":"        self._image_aging_image_marked_for_deletion()"},{"line_number":1646,"context_line":"        all_instances \u003d []"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6TE%3D","line":1643,"in_reply_to":"AAAAVn%2F%2B6kE%3D","updated":"2014-03-11 11:37:47.000000000","message":"Done","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"}],"nova/tests/virt/vmwareapi/test_imagecache.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e34aa2765a61b8737bef5eda73f632fc285a64f0","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    def setUp(self):"},{"line_number":34,"context_line":"        super(ImageCacheManagerTestCase, self).setUp()"},{"line_number":35,"context_line":"        self._session \u003d mock.Mock(name\u003d\u0027session\u0027)"},{"line_number":36,"context_line":"        self._session.i_get_vim.return_value \u003d None"},{"line_number":37,"context_line":"        self._imagecache \u003d imagecache.ImageCacheManager(self._session,"},{"line_number":38,"context_line":"                                                        \u0027fake-base-folder\u0027)"},{"line_number":39,"context_line":"        self._time \u003d datetime.datetime(2012, 11, 22, 12, 00, 00)"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6gU%3D","line":36,"updated":"2014-03-11 11:08:10.000000000","message":"Typo? If so, I guess this isn\u0027t required.","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0e1e8112f6bd210c8bd28e95444d45a3deb5531b","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    def setUp(self):"},{"line_number":34,"context_line":"        super(ImageCacheManagerTestCase, self).setUp()"},{"line_number":35,"context_line":"        self._session \u003d mock.Mock(name\u003d\u0027session\u0027)"},{"line_number":36,"context_line":"        self._session.i_get_vim.return_value \u003d None"},{"line_number":37,"context_line":"        self._imagecache \u003d imagecache.ImageCacheManager(self._session,"},{"line_number":38,"context_line":"                                                        \u0027fake-base-folder\u0027)"},{"line_number":39,"context_line":"        self._time \u003d datetime.datetime(2012, 11, 22, 12, 00, 00)"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6R4%3D","line":36,"in_reply_to":"AAAAVn%2F%2B6gU%3D","updated":"2014-03-11 11:37:47.000000000","message":"Done","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e34aa2765a61b8737bef5eda73f632fc285a64f0","unresolved":false,"context_lines":[{"line_number":159,"context_line":"            return \u0027fake-ds-browser\u0027"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        def fake_get_timestamps(ds_browser, path):"},{"line_number":162,"context_line":"            self._get_timestamp_called +\u003d 1"},{"line_number":163,"context_line":"            timestamps \u003d set()"},{"line_number":164,"context_line":"            if path \u003d\u003d \u0027fake-ds-path/fake-image-1\u0027:"},{"line_number":165,"context_line":"                # No time stamp exists"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6Zw%3D","line":162,"updated":"2014-03-11 11:08:10.000000000","message":"Nit: mock already counts this for you.","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0e1e8112f6bd210c8bd28e95444d45a3deb5531b","unresolved":false,"context_lines":[{"line_number":159,"context_line":"            return \u0027fake-ds-browser\u0027"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        def fake_get_timestamps(ds_browser, path):"},{"line_number":162,"context_line":"            self._get_timestamp_called +\u003d 1"},{"line_number":163,"context_line":"            timestamps \u003d set()"},{"line_number":164,"context_line":"            if path \u003d\u003d \u0027fake-ds-path/fake-image-1\u0027:"},{"line_number":165,"context_line":"                # No time stamp exists"}],"source_content_type":"text/x-python","patch_set":71,"id":"AAAAVn%2F%2B6Nk%3D","line":162,"in_reply_to":"AAAAVn%2F%2B6Zw%3D","updated":"2014-03-11 11:37:47.000000000","message":"i have been unable to get this to work with call_count. i think that when we override the object we need to do this ourselves","commit_id":"d71a7fa8bb4ec0d1e0d6aae97b816c7ea0f64562"}],"nova/virt/vmwareapi/driver.py":[{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"815331487e6d1d026e34c73b91be508a53d7ea07","unresolved":false,"context_lines":[{"line_number":731,"context_line":"        # Running instances per cluster"},{"line_number":732,"context_line":"        cluster_instances \u003d {}"},{"line_number":733,"context_line":"        for instance in all_instances:"},{"line_number":734,"context_line":"            instances \u003d cluster_instances.get(instance[\u0027node\u0027])"},{"line_number":735,"context_line":"            if instances:"},{"line_number":736,"context_line":"                instances.append(instance)"},{"line_number":737,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAUH%2F9oXA%3D","line":734,"updated":"2014-02-05 14:48:25.000000000","message":"nit: I think you can replace lines 734-739 with: \n\n    cluster_instances.setdefault(instance[\u0027node\u0027], []).append(instance)","commit_id":"4ac190c02f3c3da482fc68a28c6cdf844cdee5d7"},{"author":{"_account_id":8119,"name":"Eric Brown","email":"eric_wade_brown@yahoo.com","username":"ericwb"},"change_message_id":"dfffbe1fc8841b900f16200a2fcd68dc1eab85f7","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    \"\"\"The ESX host connection object.\"\"\""},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    capabilities \u003d {"},{"line_number":99,"context_line":"        \"has_imagecache\": True,"},{"line_number":100,"context_line":"        }"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    # VMwareAPI has both ESXi and vCenter API sets."}],"source_content_type":"text/x-python","patch_set":33,"id":"AAAAUH%2F9U5U%3D","line":99,"updated":"2014-02-07 18:12:36.000000000","message":"Trailing comma isn\u0027t required.","commit_id":"5bc966d57b1c56303b65e0ba824d1e86153f4705"}],"nova/virt/vmwareapi/ds_util.py":[{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"815331487e6d1d026e34c73b91be508a53d7ea07","unresolved":false,"context_lines":[{"line_number":47,"context_line":"def file_delete(session, instance, datastore_path, dc_ref):"},{"line_number":48,"context_line":"    LOG.debug(_(\"Deleting the datastore file %s\"), datastore_path,"},{"line_number":49,"context_line":"              instance\u003dinstance)"},{"line_number":50,"context_line":"    if instance:"},{"line_number":51,"context_line":"        uuid \u003d instance[\u0027uuid\u0027]"},{"line_number":52,"context_line":"    else:"},{"line_number":53,"context_line":"        uuid \u003d uuidutils.generate_uuid()"}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAUH%2F9oWA%3D","line":50,"updated":"2014-02-05 14:48:25.000000000","message":"consider:  uuid \u003d instance.get(\u0027uuid\u0027, uuidutils.generate_uuid())","commit_id":"4ac190c02f3c3da482fc68a28c6cdf844cdee5d7"},{"author":{"_account_id":8119,"name":"Eric Brown","email":"eric_wade_brown@yahoo.com","username":"ericwb"},"change_message_id":"dfffbe1fc8841b900f16200a2fcd68dc1eab85f7","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    LOG.debug(_(\"Deleting the datastore file %s\"), datastore_path,"},{"line_number":48,"context_line":"              instance\u003dinstance)"},{"line_number":49,"context_line":"    if instance:"},{"line_number":50,"context_line":"        uuid \u003d instance[\u0027uuid\u0027]"},{"line_number":51,"context_line":"    else:"},{"line_number":52,"context_line":"        uuid \u003d None"},{"line_number":53,"context_line":"    vim \u003d session._get_vim()"}],"source_content_type":"text/x-python","patch_set":33,"id":"AAAAUH%2F9U6M%3D","line":50,"updated":"2014-02-07 18:12:36.000000000","message":"You could simplify with: uuid \u003d instance[\u0027uuid\u0027] if instance else None","commit_id":"5bc966d57b1c56303b65e0ba824d1e86153f4705"},{"author":{"_account_id":8119,"name":"Eric Brown","email":"eric_wade_brown@yahoo.com","username":"ericwb"},"change_message_id":"ea7d1a06bc331de6e87e0e69ef2bdfcf417fc0c9","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    # populate the cached images"},{"line_number":147,"context_line":"    if hasattr(task_info.result, \u0027file\u0027):"},{"line_number":148,"context_line":"        for file in task_info.result.file:"},{"line_number":149,"context_line":"            folders.add(file.path)"},{"line_number":150,"context_line":"    return folders"}],"source_content_type":"text/x-python","patch_set":39,"id":"AAAAUX%2F%2FmLg%3D","line":149,"updated":"2014-02-12 05:28:12.000000000","message":"You could use a list comprehension here:\n[folders.add(file.path) for file in task_info.result.file]","commit_id":"24dfac9849743ffc5828bdc30e5087039eef19ed"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    LOG.debug(_(\"File moved\"))"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def path_exists(session, ds_browser, ds_path):"},{"line_number":98,"context_line":"    \"\"\"Check if the path exists on the datastore.\"\"\""},{"line_number":99,"context_line":"    search_task \u003d session._call_method("},{"line_number":100,"context_line":"            session._get_vim(),"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FrTA%3D","line":97,"updated":"2014-03-04 19:09:44.000000000","message":"This function isn\u0027t required here.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    LOG.debug(_(\"File moved\"))"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def path_exists(session, ds_browser, ds_path):"},{"line_number":98,"context_line":"    \"\"\"Check if the path exists on the datastore.\"\"\""},{"line_number":99,"context_line":"    search_task \u003d session._call_method("},{"line_number":100,"context_line":"            session._get_vim(),"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FqoQ%3D","line":97,"in_reply_to":"AAAAVn%2F%2FrTA%3D","updated":"2014-03-05 02:06:38.000000000","message":"Done","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":140,"context_line":"    LOG.debug(_(\"Created directory with path %s\"), ds_path)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"def get_sub_folders(session, ds_browser, ds_path):"},{"line_number":144,"context_line":"    folders \u003d set()"},{"line_number":145,"context_line":"    if not path_exists(session, ds_browser, ds_path):"},{"line_number":146,"context_line":"        return folders"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FrQ0%3D","line":143,"updated":"2014-03-04 19:09:44.000000000","message":"No docstring.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":140,"context_line":"    LOG.debug(_(\"Created directory with path %s\"), ds_path)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"def get_sub_folders(session, ds_browser, ds_path):"},{"line_number":144,"context_line":"    folders \u003d set()"},{"line_number":145,"context_line":"    if not path_exists(session, ds_browser, ds_path):"},{"line_number":146,"context_line":"        return folders"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FqpQ%3D","line":143,"in_reply_to":"AAAAVn%2F%2FrQ0%3D","updated":"2014-03-05 02:06:38.000000000","message":"Done","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":143,"context_line":"def get_sub_folders(session, ds_browser, ds_path):"},{"line_number":144,"context_line":"    folders \u003d set()"},{"line_number":145,"context_line":"    if not path_exists(session, ds_browser, ds_path):"},{"line_number":146,"context_line":"        return folders"},{"line_number":147,"context_line":"    client_factory \u003d session._get_vim().client.factory"},{"line_number":148,"context_line":"    search_task \u003d session._call_method("},{"line_number":149,"context_line":"            session._get_vim(),"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FrPk%3D","line":146,"updated":"2014-03-04 19:09:44.000000000","message":"This test isn\u0027t required.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":143,"context_line":"def get_sub_folders(session, ds_browser, ds_path):"},{"line_number":144,"context_line":"    folders \u003d set()"},{"line_number":145,"context_line":"    if not path_exists(session, ds_browser, ds_path):"},{"line_number":146,"context_line":"        return folders"},{"line_number":147,"context_line":"    client_factory \u003d session._get_vim().client.factory"},{"line_number":148,"context_line":"    search_task \u003d session._call_method("},{"line_number":149,"context_line":"            session._get_vim(),"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FoSk%3D","line":146,"in_reply_to":"AAAAVn%2F%2FrPk%3D","updated":"2014-03-05 02:06:38.000000000","message":"Done","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            \"SearchDatastore_Task\","},{"line_number":151,"context_line":"            ds_browser,"},{"line_number":152,"context_line":"            datastorePath\u003dds_path)"},{"line_number":153,"context_line":"    task_info \u003d session._wait_for_task(search_task)"},{"line_number":154,"context_line":"    # populate the cached images"},{"line_number":155,"context_line":"    if hasattr(task_info.result, \u0027file\u0027):"},{"line_number":156,"context_line":"        for file in task_info.result.file:"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FrOQ%3D","line":153,"updated":"2014-03-04 19:09:44.000000000","message":"Catch FileNotFound here to remove necessity for an extra api call and the addition of path_exists(). Use of path_exists() should be discouraged as it\u0027s rarely necessary and usually a race condition.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            \"SearchDatastore_Task\","},{"line_number":151,"context_line":"            ds_browser,"},{"line_number":152,"context_line":"            datastorePath\u003dds_path)"},{"line_number":153,"context_line":"    task_info \u003d session._wait_for_task(search_task)"},{"line_number":154,"context_line":"    # populate the cached images"},{"line_number":155,"context_line":"    if hasattr(task_info.result, \u0027file\u0027):"},{"line_number":156,"context_line":"        for file in task_info.result.file:"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FoSY%3D","line":153,"in_reply_to":"AAAAVn%2F%2FrOQ%3D","updated":"2014-03-05 02:06:38.000000000","message":"Done","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            ds_browser,"},{"line_number":152,"context_line":"            datastorePath\u003dds_path)"},{"line_number":153,"context_line":"    task_info \u003d session._wait_for_task(search_task)"},{"line_number":154,"context_line":"    # populate the cached images"},{"line_number":155,"context_line":"    if hasattr(task_info.result, \u0027file\u0027):"},{"line_number":156,"context_line":"        for file in task_info.result.file:"},{"line_number":157,"context_line":"            folders.add(file.path)"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FrSA%3D","line":154,"updated":"2014-03-04 19:09:44.000000000","message":"s/cached images/folder entries/","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            ds_browser,"},{"line_number":152,"context_line":"            datastorePath\u003dds_path)"},{"line_number":153,"context_line":"    task_info \u003d session._wait_for_task(search_task)"},{"line_number":154,"context_line":"    # populate the cached images"},{"line_number":155,"context_line":"    if hasattr(task_info.result, \u0027file\u0027):"},{"line_number":156,"context_line":"        for file in task_info.result.file:"},{"line_number":157,"context_line":"            folders.add(file.path)"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FoSI%3D","line":154,"in_reply_to":"AAAAVn%2F%2FrSA%3D","updated":"2014-03-05 02:06:38.000000000","message":"Done","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7af21c4c46cd7cf54ecca83eb75e19b47a6ad081","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        return folders"},{"line_number":145,"context_line":"    # populate the folder entries"},{"line_number":146,"context_line":"    if hasattr(task_info.result, \u0027file\u0027):"},{"line_number":147,"context_line":"        for file in task_info.result.file:"},{"line_number":148,"context_line":"            folders.add(file.path)"},{"line_number":149,"context_line":"    return folders"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FiBE%3D","line":147,"updated":"2014-03-05 15:48:25.000000000","message":"Suggestion: return set([file.path for file in task_info.result.file])","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        return folders"},{"line_number":145,"context_line":"    # populate the folder entries"},{"line_number":146,"context_line":"    if hasattr(task_info.result, \u0027file\u0027):"},{"line_number":147,"context_line":"        for file in task_info.result.file:"},{"line_number":148,"context_line":"            folders.add(file.path)"},{"line_number":149,"context_line":"    return folders"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2Fhd8%3D","line":147,"in_reply_to":"AAAAVn%2F%2FiBE%3D","updated":"2014-03-05 16:55:59.000000000","message":"Done","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"}],"nova/virt/vmwareapi/fake.py":[{"author":{"_account_id":8119,"name":"Eric Brown","email":"eric_wade_brown@yahoo.com","username":"ericwb"},"change_message_id":"dfffbe1fc8841b900f16200a2fcd68dc1eab85f7","unresolved":false,"context_lines":[{"line_number":1098,"context_line":"                if file.find(ds_path) !\u003d -1:"},{"line_number":1099,"context_line":"                    if not file.endswith(ds_path):"},{"line_number":1100,"context_line":"                        path \u003d file.lstrip(dname).split(\u0027/\u0027)"},{"line_number":1101,"context_line":"                        if len(path) \u003e 1:"},{"line_number":1102,"context_line":"                            matched_files.add(path[0])"},{"line_number":1103,"context_line":"            if not matched_files:"},{"line_number":1104,"context_line":"                matched_files.add(\u0027/\u0027)"}],"source_content_type":"text/x-python","patch_set":33,"id":"AAAAUH%2F9U74%3D","line":1101,"updated":"2014-02-07 18:12:36.000000000","message":"In Python, I believe you could simplify \"if len(path) \u003e 1:\" with just \"if path:\"","commit_id":"5bc966d57b1c56303b65e0ba824d1e86153f4705"},{"author":{"_account_id":8119,"name":"Eric Brown","email":"eric_wade_brown@yahoo.com","username":"ericwb"},"change_message_id":"ea7d1a06bc331de6e87e0e69ef2bdfcf417fc0c9","unresolved":false,"context_lines":[{"line_number":863,"context_line":"            if file.find(file_path) !\u003d -1:"},{"line_number":864,"context_line":"                to_delete.add(file)"},{"line_number":865,"context_line":"        for file in to_delete:"},{"line_number":866,"context_line":"            _db_content.get(\"files\").remove(file)"},{"line_number":867,"context_line":""},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"def fake_plug_vifs(*args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":39,"id":"AAAAUX%2F%2FmEk%3D","line":866,"updated":"2014-02-12 05:28:12.000000000","message":"Nice place to use map function.\nmap(_db_content.get(\"files\").remove, to_delete)","commit_id":"24dfac9849743ffc5828bdc30e5087039eef19ed"}],"nova/virt/vmwareapi/imagecache.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"9b4116c7e30f733cfa786be02be001650df2f6c4","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        if ts:"},{"line_number":49,"context_line":"            ts_path \u003d \u0027%s/%s\u0027 % (ds_path, ts)"},{"line_number":50,"context_line":"            LOG.debug(_(\"Timestamp path %s exists. Deleting!\"), ts_path)"},{"line_number":51,"context_line":"            ds_util.file_delete(self._session, instance, ts_path, dc_ref)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def _get_timestamp(self, ds_browser, ds_path):"},{"line_number":54,"context_line":"        # A list of files in ds_path will be returned - we need to get"}],"source_content_type":"text/x-python","patch_set":33,"id":"AAAAUH%2F9VB0%3D","line":51,"updated":"2014-02-07 17:44:59.000000000","message":"catch exception if the file does not exist - it may be deleted by another process","commit_id":"5bc966d57b1c56303b65e0ba824d1e86153f4705"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"01f944c3713e993fdc58ed373383dc231f538683","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                if not ts:"},{"line_number":108,"context_line":"                    ts_path \u003d \u0027%s/%s\u0027 % (path,"},{"line_number":109,"context_line":"                                         self._get_timestamp_filename())"},{"line_number":110,"context_line":"                    ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":111,"context_line":"                    LOG.debug(_(\"Image %s is no longer used by this node. \""},{"line_number":112,"context_line":"                                \"Pending deletion!\"), image)"},{"line_number":113,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":33,"id":"AAAAUH%2F9U%2FI%3D","line":110,"updated":"2014-02-07 17:47:24.000000000","message":"catch exceptions","commit_id":"5bc966d57b1c56303b65e0ba824d1e86153f4705"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0d0cd4b4931c02e6e7dafda4572374d12183172a","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                    if timeutils.is_older_than(dt, age_seconds):"},{"line_number":116,"context_line":"                        LOG.info(_(\"Image %s is no longer used. \""},{"line_number":117,"context_line":"                                   \"Deleting!\"), path)"},{"line_number":118,"context_line":"                        ds_util.file_delete(self._session, None, path,"},{"line_number":119,"context_line":"                                            dc_info.ref)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        # Multi node support - if the image is used and the timestamp file"}],"source_content_type":"text/x-python","patch_set":33,"id":"AAAAUH%2F9U%2Fs%3D","line":118,"updated":"2014-02-07 17:46:36.000000000","message":"catch exception - file may have been deleted by another process","commit_id":"5bc966d57b1c56303b65e0ba824d1e86153f4705"},{"author":{"_account_id":8119,"name":"Eric Brown","email":"eric_wade_brown@yahoo.com","username":"ericwb"},"change_message_id":"ea7d1a06bc331de6e87e0e69ef2bdfcf417fc0c9","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        files \u003d ds_util.get_sub_folders(self._session, ds_browser, ds_path)"},{"line_number":71,"context_line":"        if files:"},{"line_number":72,"context_line":"            for file in files:"},{"line_number":73,"context_line":"                if file.startswith(TIMESTAMP_PREFIX):"},{"line_number":74,"context_line":"                    return file"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def _get_timestamp_filename(self):"}],"source_content_type":"text/x-python","patch_set":39,"id":"AAAAUX%2F%2FmIY%3D","line":73,"updated":"2014-02-12 05:28:12.000000000","message":"return next((file for file in files if file.startswith(TIMESTAMP_PREFIX)), None)","commit_id":"24dfac9849743ffc5828bdc30e5087039eef19ed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f32883038611e0aca4a8feb228daee047a44778e","unresolved":false,"context_lines":[{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"File already exists\"))"},{"line_number":128,"context_line":"                    LOG.info(_(\"Image %s is no longer used by this node. \""},{"line_number":129,"context_line":"                               \"Pending deletion!\"), image)"},{"line_number":130,"context_line":"                else:"},{"line_number":131,"context_line":"                    dt \u003d self._get_datetime_from_filename(ts)"},{"line_number":132,"context_line":"                    if timeutils.is_older_than(dt, age_seconds):"}],"source_content_type":"text/x-python","patch_set":43,"id":"AAAAUX%2F%2B91Q%3D","line":129,"updated":"2014-02-17 23:10:00.000000000","message":"Isn\u0027t this a giant race? If a thousand nodes (with unsyncrhonized clocks) hit this code at the same time on shared storage, won\u0027t a bunch of them create a different timestamp file?\n\nWouldn\u0027t it be much simpler to use a predictable timestamp filename that we can use to signal whether the image is walking dead and then stash the expiration date inside? That would avoid the goofy list-all-sub-folders-and-see-if-one-is-a-timestamp loop above.\n\nIt also seems really bizarre to use at timestamp *directory*...","commit_id":"acbaa114703460c61bc06948cd7874b626c4580d"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b8375bb70027f5be1ffca7408bf69d3e889050dc","unresolved":false,"context_lines":[{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"File already exists\"))"},{"line_number":128,"context_line":"                    LOG.info(_(\"Image %s is no longer used by this node. \""},{"line_number":129,"context_line":"                               \"Pending deletion!\"), image)"},{"line_number":130,"context_line":"                else:"},{"line_number":131,"context_line":"                    dt \u003d self._get_datetime_from_filename(ts)"},{"line_number":132,"context_line":"                    if timeutils.is_older_than(dt, age_seconds):"}],"source_content_type":"text/x-python","patch_set":43,"id":"AAAAUX%2F%2B4dc%3D","line":129,"in_reply_to":"AAAAUX%2F%2B91Q%3D","updated":"2014-02-18 12:51:38.000000000","message":"There are 2 issues here.\n1. All of the compute nodes need to be synced regarding their times. I recall in the past being unable to add a compute node to a OpenStack setup if the times were not in sync. My general understanding is that they should be using a NTP server to make sure that the times are in sync.\n2. The reason that we chose to the use the subdirectory name was that there is a specific VMware API for this - that is the creation of a directory. Managing files as you have suggested is problematic. There are a number of reasons for this:\ni. there is no supported API - so we will have to write code that will upload a file\nii. different datastores types may have different interfaces\n\nIn addition to this I am not sure that each compute node needs to do the aging for a common backend datastore. That is, one can be configured to do the aging.\n\nIn paralellel we are working on a way to try and have a \u0027shared\u0027 lock that will address the concerns that you have mentioned. Hopefully that will not block this.","commit_id":"acbaa114703460c61bc06948cd7874b626c4580d"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"91e1910538ab95ff83eafe172fc6bd63b48a2517","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                                         self._get_timestamp_filename())"},{"line_number":124,"context_line":"                    try:"},{"line_number":125,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"File already exists\"))"},{"line_number":128,"context_line":"                    LOG.info(_(\"Image %s is no longer used by this node. \""},{"line_number":129,"context_line":"                               \"Pending deletion!\"), image)"}],"source_content_type":"text/x-python","patch_set":51,"id":"AAAAVX%2F%2F848%3D","line":126,"updated":"2014-02-27 07:04:40.000000000","message":"Does this really raise FileAlreadyExists and not DirectyAlreadyExists?","commit_id":"82fc8cf881633c179338c00874283b09705f09f4"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"91e1910538ab95ff83eafe172fc6bd63b48a2517","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                    try:"},{"line_number":125,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"File already exists\"))"},{"line_number":128,"context_line":"                    LOG.info(_(\"Image %s is no longer used by this node. \""},{"line_number":129,"context_line":"                               \"Pending deletion!\"), image)"},{"line_number":130,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":51,"id":"AAAAVX%2F%2F84w%3D","line":127,"updated":"2014-02-27 07:04:40.000000000","message":"directory?","commit_id":"82fc8cf881633c179338c00874283b09705f09f4"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"6f4cef3df753486573a3e3c2bcc7a56d429f86b8","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                                         self._get_timestamp_filename())"},{"line_number":124,"context_line":"                    try:"},{"line_number":125,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"File already exists\"))"},{"line_number":128,"context_line":"                    LOG.info(_(\"Image %s is no longer used by this node. \""},{"line_number":129,"context_line":"                               \"Pending deletion!\"), image)"}],"source_content_type":"text/x-python","patch_set":54,"id":"AAAAVX%2F%2FzvY%3D","line":126,"updated":"2014-02-28 05:56:24.000000000","message":"Does this really raise FileAlreadyExists and not DirectyAlreadyExists?","commit_id":"510dee0389074d2f561e3e7cc2a9a6b7a92b7520"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0dff44882f1a02203e779d5a2d3433de19846d3b","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                                         self._get_timestamp_filename())"},{"line_number":124,"context_line":"                    try:"},{"line_number":125,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"File already exists\"))"},{"line_number":128,"context_line":"                    LOG.info(_(\"Image %s is no longer used by this node. \""},{"line_number":129,"context_line":"                               \"Pending deletion!\"), image)"}],"source_content_type":"text/x-python","patch_set":54,"id":"AAAAVX%2F%2Fuws%3D","line":126,"in_reply_to":"AAAAVX%2F%2FzvY%3D","updated":"2014-02-28 17:58:43.000000000","message":"Yes, this is the exception that is raised by the backend.","commit_id":"510dee0389074d2f561e3e7cc2a9a6b7a92b7520"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"6f4cef3df753486573a3e3c2bcc7a56d429f86b8","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                    try:"},{"line_number":125,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"File already exists\"))"},{"line_number":128,"context_line":"                    LOG.info(_(\"Image %s is no longer used by this node. \""},{"line_number":129,"context_line":"                               \"Pending deletion!\"), image)"},{"line_number":130,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":54,"id":"AAAAVX%2F%2FzvQ%3D","line":127,"updated":"2014-02-28 05:56:24.000000000","message":"directory?","commit_id":"510dee0389074d2f561e3e7cc2a9a6b7a92b7520"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0dff44882f1a02203e779d5a2d3433de19846d3b","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                    try:"},{"line_number":125,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"File already exists\"))"},{"line_number":128,"context_line":"                    LOG.info(_(\"Image %s is no longer used by this node. \""},{"line_number":129,"context_line":"                               \"Pending deletion!\"), image)"},{"line_number":130,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":54,"id":"AAAAVX%2F%2Fuwk%3D","line":127,"in_reply_to":"AAAAVX%2F%2FzvQ%3D","updated":"2014-02-28 17:58:43.000000000","message":"Done","commit_id":"510dee0389074d2f561e3e7cc2a9a6b7a92b7520"},{"author":{"_account_id":7400,"name":"Tracy Jones","email":"tjones@vmware.com","username":"tracyajones"},"change_message_id":"462909e0ba7f8799eda61b5720ae95921e9de361","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        ts \u003d timestamp_filename.lstrip(TIMESTAMP_PREFIX)"},{"line_number":82,"context_line":"        return timeutils.parse_strtime(ts, fmt\u003dTIMESTAMP_FORMAT)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def _get_ds_browser(self, ds_ref):"},{"line_number":85,"context_line":"        ds_browser \u003d self._ds_browser.get(ds_ref)"},{"line_number":86,"context_line":"        if not ds_browser:"},{"line_number":87,"context_line":"            ds_browser \u003d vim_util.get_dynamic_property("}],"source_content_type":"text/x-python","patch_set":55,"id":"AAAAVn%2F%2F3lI%3D","line":84,"updated":"2014-03-03 18:11:44.000000000","message":"we have this in vmops already - can this move to a utility method?","commit_id":"2d86f25c946fdeeca7c4b6c61324cc15f1fdd90b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d768796c098ee30486db9b69e8438df48dfaba81","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        ts \u003d timestamp_filename.lstrip(TIMESTAMP_PREFIX)"},{"line_number":82,"context_line":"        return timeutils.parse_strtime(ts, fmt\u003dTIMESTAMP_FORMAT)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def _get_ds_browser(self, ds_ref):"},{"line_number":85,"context_line":"        ds_browser \u003d self._ds_browser.get(ds_ref)"},{"line_number":86,"context_line":"        if not ds_browser:"},{"line_number":87,"context_line":"            ds_browser \u003d vim_util.get_dynamic_property("}],"source_content_type":"text/x-python","patch_set":55,"id":"AAAAVn%2F%2F2fs%3D","line":84,"in_reply_to":"AAAAVn%2F%2F3lI%3D","updated":"2014-03-04 14:33:20.000000000","message":"maybe we can do that in an additional patch. basically this utility method would be just wrapping lines 87-98\nthis is also only called once per ds_ref - the data is cached from that moment on.","commit_id":"2d86f25c946fdeeca7c4b6c61324cc15f1fdd90b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":90,"context_line":"            self._ds_browser[ds_ref] \u003d ds_browser"},{"line_number":91,"context_line":"        return ds_browser"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _get_base(self, datastore):"},{"line_number":94,"context_line":"        \"\"\"Returns the base directory of the cached images.\"\"\""},{"line_number":95,"context_line":"        ds_path \u003d ds_util.build_datastore_path(datastore[\u0027name\u0027],"},{"line_number":96,"context_line":"                                               self._base_folder)"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FqwQ%3D","line":93,"updated":"2014-03-04 19:09:44.000000000","message":"Different signature to the superclass.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":90,"context_line":"            self._ds_browser[ds_ref] \u003d ds_browser"},{"line_number":91,"context_line":"        return ds_browser"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _get_base(self, datastore):"},{"line_number":94,"context_line":"        \"\"\"Returns the base directory of the cached images.\"\"\""},{"line_number":95,"context_line":"        ds_path \u003d ds_util.build_datastore_path(datastore[\u0027name\u0027],"},{"line_number":96,"context_line":"                                               self._base_folder)"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FqYM%3D","line":93,"in_reply_to":"AAAAVn%2F%2FqwQ%3D","updated":"2014-03-05 02:06:38.000000000","message":"This is required as we are passing different variable to those used by libvirt. In this case there may be a number of different datastores.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                                               self._base_folder)"},{"line_number":97,"context_line":"        return ds_path"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def _list_base_images(self, ds_path, datastore):"},{"line_number":100,"context_line":"        \"\"\"Return a list of the images present in _base."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        This method returns a dictionary with the following keys:"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FrCA%3D","line":99,"updated":"2014-03-04 19:09:44.000000000","message":"This has a different signature to the superclass, which seems unlikely to work. If it does, we need a big comment here explaining how.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                                               self._base_folder)"},{"line_number":97,"context_line":"        return ds_path"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def _list_base_images(self, ds_path, datastore):"},{"line_number":100,"context_line":"        \"\"\"Return a list of the images present in _base."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        This method returns a dictionary with the following keys:"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FqWQ%3D","line":99,"in_reply_to":"AAAAVn%2F%2FrCA%3D","updated":"2014-03-05 02:06:38.000000000","message":"Done","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                \u0027originals\u0027: originals}"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def _age_and_verify_cached_images(self, context, datastore, dc_info,"},{"line_number":113,"context_line":"                                      ds_path):"},{"line_number":114,"context_line":"        age_seconds \u003d CONF.remove_unused_original_minimum_age_seconds"},{"line_number":115,"context_line":"        unused_images \u003d self.originals - self.used_images"},{"line_number":116,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2Fq4g%3D","line":113,"updated":"2014-03-04 19:09:44.000000000","message":"Signature differs from the superclass, which doesn\u0027t look safe. Needs a big comment at the very least.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                \u0027originals\u0027: originals}"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def _age_and_verify_cached_images(self, context, datastore, dc_info,"},{"line_number":113,"context_line":"                                      ds_path):"},{"line_number":114,"context_line":"        age_seconds \u003d CONF.remove_unused_original_minimum_age_seconds"},{"line_number":115,"context_line":"        unused_images \u003d self.originals - self.used_images"},{"line_number":116,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FpkQ%3D","line":113,"in_reply_to":"AAAAVn%2F%2Fq4g%3D","updated":"2014-03-05 02:06:38.000000000","message":"Done","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":112,"context_line":"    def _age_and_verify_cached_images(self, context, datastore, dc_info,"},{"line_number":113,"context_line":"                                      ds_path):"},{"line_number":114,"context_line":"        age_seconds \u003d CONF.remove_unused_original_minimum_age_seconds"},{"line_number":115,"context_line":"        unused_images \u003d self.originals - self.used_images"},{"line_number":116,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"},{"line_number":117,"context_line":"        if unused_images:"},{"line_number":118,"context_line":"            for image in unused_images:"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2Fq8g%3D","line":115,"updated":"2014-03-04 19:09:44.000000000","message":"self.originals and self.used_images are set in updates(), not in __init__(). It doesn\u0027t look to me as though update() is necessarily called directly after ImageCacheManager creation, so if anything expecting the interface of the superclass tries to call this it will:\n\n1. explode because of an incorrect signature.\n2. explode because of missing attributes.\n\nIf _age_and_verify_cached_images() is truly a private interface I guess this doesn\u0027t matter, but in that case why would it be in the superclass?","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":112,"context_line":"    def _age_and_verify_cached_images(self, context, datastore, dc_info,"},{"line_number":113,"context_line":"                                      ds_path):"},{"line_number":114,"context_line":"        age_seconds \u003d CONF.remove_unused_original_minimum_age_seconds"},{"line_number":115,"context_line":"        unused_images \u003d self.originals - self.used_images"},{"line_number":116,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"},{"line_number":117,"context_line":"        if unused_images:"},{"line_number":118,"context_line":"            for image in unused_images:"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FpjA%3D","line":115,"in_reply_to":"AAAAVn%2F%2Fq8g%3D","updated":"2014-03-05 02:06:38.000000000","message":"The method _list_base_images creates the set - originals. That is a set of images that has been cached.\n\nI am not sure what you mean by explode. python has its own garbage collection and we do not need to do this. This set is created once every aging interation with is by default once every 24 hours). please see the method update below to follow the flow.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        age_seconds \u003d CONF.remove_unused_original_minimum_age_seconds"},{"line_number":115,"context_line":"        unused_images \u003d self.originals - self.used_images"},{"line_number":116,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"},{"line_number":117,"context_line":"        if unused_images:"},{"line_number":118,"context_line":"            for image in unused_images:"},{"line_number":119,"context_line":"                path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":120,"context_line":"                ts \u003d self._get_timestamp(ds_browser, path)"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FqxE%3D","line":117,"updated":"2014-03-04 19:09:44.000000000","message":"Nit: Not required, as unused_images can\u0027t be None, and if it\u0027s the empty set the for loop will handle it.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        if unused_images:"},{"line_number":118,"context_line":"            for image in unused_images:"},{"line_number":119,"context_line":"                path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":120,"context_line":"                ts \u003d self._get_timestamp(ds_browser, path)"},{"line_number":121,"context_line":"                if not ts:"},{"line_number":122,"context_line":"                    ts_path \u003d \u0027%s/%s\u0027 % (path,"},{"line_number":123,"context_line":"                                         self._get_timestamp_filename())"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FqvA%3D","line":120,"updated":"2014-03-04 19:09:44.000000000","message":"From here...","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        if unused_images:"},{"line_number":118,"context_line":"            for image in unused_images:"},{"line_number":119,"context_line":"                path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":120,"context_line":"                ts \u003d self._get_timestamp(ds_browser, path)"},{"line_number":121,"context_line":"                if not ts:"},{"line_number":122,"context_line":"                    ts_path \u003d \u0027%s/%s\u0027 % (path,"},{"line_number":123,"context_line":"                                         self._get_timestamp_filename())"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FoOc%3D","line":120,"in_reply_to":"AAAAVn%2F%2FqvA%3D","updated":"2014-03-05 02:06:38.000000000","message":"please see the comment below","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                    ts_path \u003d \u0027%s/%s\u0027 % (path,"},{"line_number":123,"context_line":"                                         self._get_timestamp_filename())"},{"line_number":124,"context_line":"                    try:"},{"line_number":125,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"Directory already exists. Timestamp \""},{"line_number":128,"context_line":"                                    \"created by another node.\"))"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FqtU%3D","line":125,"updated":"2014-03-04 19:09:44.000000000","message":"...to here is a race. You could end up with 2 timestamp directories.\n\nThis is going to require a bit of thinking to work round. I think the solution will involve having a predictable file stamp name, and somehow using its contents.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                    ts_path \u003d \u0027%s/%s\u0027 % (path,"},{"line_number":123,"context_line":"                                         self._get_timestamp_filename())"},{"line_number":124,"context_line":"                    try:"},{"line_number":125,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"Directory already exists. Timestamp \""},{"line_number":128,"context_line":"                                    \"created by another node.\"))"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2Fn3I%3D","line":125,"in_reply_to":"AAAAVn%2F%2Fn38%3D","updated":"2014-03-05 02:06:38.000000000","message":"Following Vui\u0027s comment I am going to ensure that the cleanup cleans up all timestamp folders in a specific datastore path.\n\nThe algorithm is as follows:\nif the image is not used by any instances then it will be marked for deletion. marked means that there will be a timestamp folder in the image directory. if there is a instance launched then the timestamp will be deleted.\nso if we have more than one timestamp directory then we should delete them all\nwhen the image is to be aged the parent folder will be deleted","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"6ca2026213d197223f0c3d52c85ba145c6e94cb3","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                    ts_path \u003d \u0027%s/%s\u0027 % (path,"},{"line_number":123,"context_line":"                                         self._get_timestamp_filename())"},{"line_number":124,"context_line":"                    try:"},{"line_number":125,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":126,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":127,"context_line":"                        LOG.debug(_(\"Directory already exists. Timestamp \""},{"line_number":128,"context_line":"                                    \"created by another node.\"))"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2Fn38%3D","line":125,"in_reply_to":"AAAAVn%2F%2FqtU%3D","updated":"2014-03-05 00:54:31.000000000","message":"In fact there can possibly be N directories created in a race condition if there are N compute nodes using the same image/datastore.\n\nIt would be nice to use a single file (or just use a file at all instead of directory), but I think Gary has explained the messiness with the file based approach given the limitations of the vSphere API.\n\nFinally, the race is not that big of a problem, we just have to account for the possibility of seeing multiple ts directories. I see an issue currently where we do not delete _all_ timestamp directories of an image when it found to be in use again. Once we fix that we should be good, I think.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7af21c4c46cd7cf54ecca83eb75e19b47a6ad081","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"\"\"\""},{"line_number":16,"context_line":"Image cache class"},{"line_number":17,"context_line":"\"\"\""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from oslo.config import cfg"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2Fkgo%3D","line":17,"updated":"2014-03-05 15:48:25.000000000","message":"We need a big comment here explaining how the cache manager works. Essentially an architecture description. It should include a detailed description of how and when timestamp directories should be created and removed.\n\nThis isn\u0027t useful just for the reviewer, but also for the future maintainer. Right now you have to read the code, infer the design, verify the design, then re-read the code to ensure it actually does what it looked like it was trying to do. A link to the BP wouldn\u0027t be sufficient here, as the description should evolve with the code.\n\nThings which I have found non-obvious include handling of multiple timestamps, and the reliance on Nova time.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"\"\"\""},{"line_number":16,"context_line":"Image cache class"},{"line_number":17,"context_line":"\"\"\""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from oslo.config import cfg"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FhT8%3D","line":17,"in_reply_to":"AAAAVn%2F%2Fkgo%3D","updated":"2014-03-05 16:55:59.000000000","message":"Done","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7af21c4c46cd7cf54ecca83eb75e19b47a6ad081","unresolved":false,"context_lines":[{"line_number":36,"context_line":"TIMESTAMP_FORMAT \u003d \u0027%Y-%m-%d-%H-%M-%S\u0027"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class ImageCacheManager(imagecache.ImageCacheManager):"},{"line_number":40,"context_line":"    def __init__(self, session, base_folder):"},{"line_number":41,"context_line":"        super(ImageCacheManager, self).__init__()"},{"line_number":42,"context_line":"        self._session \u003d session"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2Fh38%3D","line":39,"updated":"2014-03-05 15:48:25.000000000","message":"In terms of subclassing ImageCacheManager, the only common point seems to be _list_running_instances. Apart from that, Nova interfaces with the image cache via driver.manage_image_cache(), so ImageCacheManager isn\u0027t at all relevant. I would actually call it a distraction.\n\nIdeally, I would be inclined to move _list_running_instances elsewhere and entirely remove the base class. If that isn\u0027t practical for the moment, I would at least put a big comment at the top explaining that the base class isn\u0027t actually used, so any strange apparent uses of it can be ignored, with the exception of _list_running_instances.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":36,"context_line":"TIMESTAMP_FORMAT \u003d \u0027%Y-%m-%d-%H-%M-%S\u0027"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class ImageCacheManager(imagecache.ImageCacheManager):"},{"line_number":40,"context_line":"    def __init__(self, session, base_folder):"},{"line_number":41,"context_line":"        super(ImageCacheManager, self).__init__()"},{"line_number":42,"context_line":"        self._session \u003d session"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FhTM%3D","line":39,"in_reply_to":"AAAAVn%2F%2Fh38%3D","updated":"2014-03-05 16:55:59.000000000","message":"I have made minor changes to make it better.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7af21c4c46cd7cf54ecca83eb75e19b47a6ad081","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        self._base_folder \u003d base_folder"},{"line_number":44,"context_line":"        self._ds_browser \u003d {}"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _timestamp_folder_delete(self, path, dc_ref):"},{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            ds_util.file_delete(self._session, path, dc_ref)"},{"line_number":49,"context_line":"        except (error_util.CannotDeleteFileException,"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FinU%3D","line":46,"updated":"2014-03-05 15:48:25.000000000","message":"This is incorrectly named, as it\u0027s also used to delete the image folder.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        self._base_folder \u003d base_folder"},{"line_number":44,"context_line":"        self._ds_browser \u003d {}"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _timestamp_folder_delete(self, path, dc_ref):"},{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            ds_util.file_delete(self._session, path, dc_ref)"},{"line_number":49,"context_line":"        except (error_util.CannotDeleteFileException,"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FhP4%3D","line":46,"in_reply_to":"AAAAVn%2F%2FinU%3D","updated":"2014-03-05 16:55:59.000000000","message":"This actually serves two roles:\n1. delete the image folder timestamps (those are folders)\n2. delete the image id folder\n\ni have changed to _folder_delete","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7af21c4c46cd7cf54ecca83eb75e19b47a6ad081","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    def timestamp_cleanup(self, dc_ref, ds_browser,"},{"line_number":60,"context_line":"                          ds_ref, ds_name, ds_path):"},{"line_number":61,"context_line":"        timestamps \u003d self._get_timestamps(ds_browser, ds_path)"},{"line_number":62,"context_line":"        if timestamps:"},{"line_number":63,"context_line":"            for ts in timestamps:"},{"line_number":64,"context_line":"                ts_path \u003d \u0027%s/%s\u0027 % (ds_path, ts)"},{"line_number":65,"context_line":"                LOG.debug(_(\"Timestamp path %s exists. Deleting!\"), ts_path)"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FiGI%3D","line":62,"updated":"2014-03-05 15:48:25.000000000","message":"Nit: redundant test, as _get_timestamps() will at least return an empty set.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    def timestamp_cleanup(self, dc_ref, ds_browser,"},{"line_number":60,"context_line":"                          ds_ref, ds_name, ds_path):"},{"line_number":61,"context_line":"        timestamps \u003d self._get_timestamps(ds_browser, ds_path)"},{"line_number":62,"context_line":"        if timestamps:"},{"line_number":63,"context_line":"            for ts in timestamps:"},{"line_number":64,"context_line":"                ts_path \u003d \u0027%s/%s\u0027 % (ds_path, ts)"},{"line_number":65,"context_line":"                LOG.debug(_(\"Timestamp path %s exists. Deleting!\"), ts_path)"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FhLg%3D","line":62,"in_reply_to":"AAAAVn%2F%2FiGI%3D","updated":"2014-03-05 16:55:59.000000000","message":"Done","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7af21c4c46cd7cf54ecca83eb75e19b47a6ad081","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        ts \u003d timestamp_filename.lstrip(TIMESTAMP_PREFIX)"},{"line_number":87,"context_line":"        return timeutils.parse_strtime(ts, fmt\u003dTIMESTAMP_FORMAT)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def _get_ds_browser(self, ds_ref):"},{"line_number":90,"context_line":"        ds_browser \u003d self._ds_browser.get(ds_ref)"},{"line_number":91,"context_line":"        if not ds_browser:"},{"line_number":92,"context_line":"            ds_browser \u003d vim_util.get_dynamic_property("}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FiEs%3D","line":89,"updated":"2014-03-05 15:48:25.000000000","message":"With this, we now have (unmanaged) ds_browser caches in both imagecache and vmops. If this is something we\u0027re sure we need, these should be combined.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        ts \u003d timestamp_filename.lstrip(TIMESTAMP_PREFIX)"},{"line_number":87,"context_line":"        return timeutils.parse_strtime(ts, fmt\u003dTIMESTAMP_FORMAT)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def _get_ds_browser(self, ds_ref):"},{"line_number":90,"context_line":"        ds_browser \u003d self._ds_browser.get(ds_ref)"},{"line_number":91,"context_line":"        if not ds_browser:"},{"line_number":92,"context_line":"            ds_browser \u003d vim_util.get_dynamic_property("}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FhLA%3D","line":89,"in_reply_to":"AAAAVn%2F%2FiEs%3D","updated":"2014-03-05 16:55:59.000000000","message":"that is a good suggestion. i think that we can do it in a separate patch set. this basically uses a utility method below.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"d652b50ccdb647c0d86b64ac64f91b3c81dd14d3","unresolved":false,"context_lines":[{"line_number":98,"context_line":"    def _get_base(self, datastore):"},{"line_number":99,"context_line":"        \"\"\"Returns the base directory of the cached images."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        An additional parameter is added to the base class as the backend"},{"line_number":102,"context_line":"        may be connected to multiple datastores."},{"line_number":103,"context_line":"        \"\"\""},{"line_number":104,"context_line":"        ds_path \u003d ds_util.build_datastore_path(datastore[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FmPk%3D","line":101,"updated":"2014-03-05 06:11:26.000000000","message":"General comment (which also applies to all the other methods that follow):\n\nI understand what is being said here, but we should not explain the signature change as adding a parameter to a base class method.\n\nThe sameness of several of the class\u0027s method names as those of its base class notwithstanding, we really should view these methods as part of a private interface of the class and unrelated to what is implemented in the base.\n\nMy suggestion would be something like \"Unlike the identically-name method in the base class, this method takes parameters needed to connect to the \u003cright datastore\u003e / \u003csearch the right datastore\u003e / etc.\"\n\nDown the road, we can look to codify through ABCMeta exactly what methods have to be implemented in an ImageCacheManager.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":98,"context_line":"    def _get_base(self, datastore):"},{"line_number":99,"context_line":"        \"\"\"Returns the base directory of the cached images."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        An additional parameter is added to the base class as the backend"},{"line_number":102,"context_line":"        may be connected to multiple datastores."},{"line_number":103,"context_line":"        \"\"\""},{"line_number":104,"context_line":"        ds_path \u003d ds_util.build_datastore_path(datastore[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FhEg%3D","line":101,"in_reply_to":"AAAAVn%2F%2FmPk%3D","updated":"2014-03-05 16:55:59.000000000","message":"I have dropped the unnecesary overriding","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"d652b50ccdb647c0d86b64ac64f91b3c81dd14d3","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            - unexplained_images"},{"line_number":113,"context_line":"            - originals"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        An additional parameter is added to the base class as the backend"},{"line_number":116,"context_line":"        may be connected to multiple datastores."},{"line_number":117,"context_line":"        \"\"\""},{"line_number":118,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FmEw%3D","line":115,"updated":"2014-03-05 06:11:26.000000000","message":"ditto. In fact two parameters differ here.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            - unexplained_images"},{"line_number":113,"context_line":"            - originals"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        An additional parameter is added to the base class as the backend"},{"line_number":116,"context_line":"        may be connected to multiple datastores."},{"line_number":117,"context_line":"        \"\"\""},{"line_number":118,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FhJ8%3D","line":115,"in_reply_to":"AAAAVn%2F%2FmEw%3D","updated":"2014-03-05 16:55:59.000000000","message":"Done","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7af21c4c46cd7cf54ecca83eb75e19b47a6ad081","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                \u0027originals\u0027: originals}"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _age_and_verify_cached_images(self, context, datastore, dc_info,"},{"line_number":125,"context_line":"                                      ds_path):"},{"line_number":126,"context_line":"        \"\"\"Ages and verfies cached images."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        Base class parameters are replaced by those which enable the driver"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FiDM%3D","line":125,"updated":"2014-03-05 15:48:25.000000000","message":"It seems that this function is never actually called externally. If it were, it would fail anyway as it has the wrong signature. To avoid confusion, I would entirely change its name so that it doesn\u0027t appear to override the superclass. I would also pass in all its parameters, including originals and used_images, rather than storing them in the base object and retrieving them.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                \u0027originals\u0027: originals}"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _age_and_verify_cached_images(self, context, datastore, dc_info,"},{"line_number":125,"context_line":"                                      ds_path):"},{"line_number":126,"context_line":"        \"\"\"Ages and verfies cached images."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        Base class parameters are replaced by those which enable the driver"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FhEM%3D","line":125,"in_reply_to":"AAAAVn%2F%2FiDM%3D","updated":"2014-03-05 16:55:59.000000000","message":"Done","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"d652b50ccdb647c0d86b64ac64f91b3c81dd14d3","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _age_and_verify_cached_images(self, context, datastore, dc_info,"},{"line_number":125,"context_line":"                                      ds_path):"},{"line_number":126,"context_line":"        \"\"\"Ages and verfies cached images."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        Base class parameters are replaced by those which enable the driver"},{"line_number":129,"context_line":"        to access the relevant datastore paths."}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2Fmco%3D","line":126,"updated":"2014-03-05 06:11:26.000000000","message":"verfies -\u003e verifies.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _age_and_verify_cached_images(self, context, datastore, dc_info,"},{"line_number":125,"context_line":"                                      ds_path):"},{"line_number":126,"context_line":"        \"\"\"Ages and verfies cached images."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        Base class parameters are replaced by those which enable the driver"},{"line_number":129,"context_line":"        to access the relevant datastore paths."}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2Fg8E%3D","line":126,"in_reply_to":"AAAAVn%2F%2Fmco%3D","updated":"2014-03-05 16:55:59.000000000","message":"Done","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"d652b50ccdb647c0d86b64ac64f91b3c81dd14d3","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                                      ds_path):"},{"line_number":126,"context_line":"        \"\"\"Ages and verfies cached images."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        Base class parameters are replaced by those which enable the driver"},{"line_number":129,"context_line":"        to access the relevant datastore paths."},{"line_number":130,"context_line":"        \"\"\""},{"line_number":131,"context_line":"        age_seconds \u003d CONF.remove_unused_original_minimum_age_seconds"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FmEY%3D","line":128,"updated":"2014-03-05 06:11:26.000000000","message":"see general comment above. Although this one I am oddly more okay with.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                                      ds_path):"},{"line_number":126,"context_line":"        \"\"\"Ages and verfies cached images."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        Base class parameters are replaced by those which enable the driver"},{"line_number":129,"context_line":"        to access the relevant datastore paths."},{"line_number":130,"context_line":"        \"\"\""},{"line_number":131,"context_line":"        age_seconds \u003d CONF.remove_unused_original_minimum_age_seconds"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FhEE%3D","line":128,"in_reply_to":"AAAAVn%2F%2FmEY%3D","updated":"2014-03-05 16:55:59.000000000","message":"Dropped as I am not suing a private method","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7af21c4c46cd7cf54ecca83eb75e19b47a6ad081","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        age_seconds \u003d CONF.remove_unused_original_minimum_age_seconds"},{"line_number":132,"context_line":"        unused_images \u003d self.originals - self.used_images"},{"line_number":133,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"},{"line_number":134,"context_line":"        if unused_images:"},{"line_number":135,"context_line":"            for image in unused_images:"},{"line_number":136,"context_line":"                path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":137,"context_line":"                timestamps \u003d self._get_timestamps(ds_browser, path)"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2Fimk%3D","line":134,"updated":"2014-03-05 15:48:25.000000000","message":"Nit: redundant test. This can be simply omitted.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        age_seconds \u003d CONF.remove_unused_original_minimum_age_seconds"},{"line_number":132,"context_line":"        unused_images \u003d self.originals - self.used_images"},{"line_number":133,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"},{"line_number":134,"context_line":"        if unused_images:"},{"line_number":135,"context_line":"            for image in unused_images:"},{"line_number":136,"context_line":"                path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":137,"context_line":"                timestamps \u003d self._get_timestamps(ds_browser, path)"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2Fg9Q%3D","line":134,"in_reply_to":"AAAAVn%2F%2Fimk%3D","updated":"2014-03-05 16:55:59.000000000","message":"Done","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7af21c4c46cd7cf54ecca83eb75e19b47a6ad081","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        # exists then this must be deleted. The timestamp would have been"},{"line_number":160,"context_line":"        # created by another node that no longer has instances running for"},{"line_number":161,"context_line":"        # the specific image."},{"line_number":162,"context_line":"        if self.used_images:"},{"line_number":163,"context_line":"            for image in self.used_images:"},{"line_number":164,"context_line":"                path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":165,"context_line":"                self.timestamp_cleanup(dc_info.ref, ds_browser,"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FiEM%3D","line":162,"updated":"2014-03-05 15:48:25.000000000","message":"Nit: also a redundant test, as this is always a set.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        # exists then this must be deleted. The timestamp would have been"},{"line_number":160,"context_line":"        # created by another node that no longer has instances running for"},{"line_number":161,"context_line":"        # the specific image."},{"line_number":162,"context_line":"        if self.used_images:"},{"line_number":163,"context_line":"            for image in self.used_images:"},{"line_number":164,"context_line":"                path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":165,"context_line":"                self.timestamp_cleanup(dc_info.ref, ds_browser,"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2Fg8M%3D","line":162,"in_reply_to":"AAAAVn%2F%2FiEM%3D","updated":"2014-03-05 16:55:59.000000000","message":"Done","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"d652b50ccdb647c0d86b64ac64f91b3c81dd14d3","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                                       path)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def update(self, context, instances, datastores_info):"},{"line_number":170,"context_line":"        \"\"\"The cache manager."},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        This will invoke the cache manager. This will update the cache"},{"line_number":173,"context_line":"        according to the defined cache management scheme. The information"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2FmDQ%3D","line":170,"updated":"2014-03-05 06:11:26.000000000","message":"Not sure if you mean to say this as the comment header, or if some words were missing.","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d24ba2e5e94943ce1161fb65a9ea6db7c7834d09","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                                       path)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def update(self, context, instances, datastores_info):"},{"line_number":170,"context_line":"        \"\"\"The cache manager."},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        This will invoke the cache manager. This will update the cache"},{"line_number":173,"context_line":"        according to the defined cache management scheme. The information"}],"source_content_type":"text/x-python","patch_set":57,"id":"AAAAVn%2F%2Fg8Q%3D","line":170,"in_reply_to":"AAAAVn%2F%2FmDQ%3D","updated":"2014-03-05 16:55:59.000000000","message":"Done","commit_id":"225867fe81ae1d7ad9d953f6f29b52ebcaa98ccb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e260d7642cf298a375831c3cd0a212ac8cc1e718","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                ts_path \u003d \u0027%s/%s\u0027 % (path,"},{"line_number":136,"context_line":"                                     self._get_timestamp_filename())"},{"line_number":137,"context_line":"                try:"},{"line_number":138,"context_line":"                    ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":139,"context_line":"                except error_util.FileAlreadyExistsException:"},{"line_number":140,"context_line":"                    LOG.debug(_(\"Timestamp already exists. Created \""},{"line_number":141,"context_line":"                                \"by another node.\"))"}],"source_content_type":"text/x-python","patch_set":60,"id":"AAAAVn%2F%2FXKc%3D","line":138,"updated":"2014-03-06 16:18:46.000000000","message":"Isn\u0027t there still a race between self._get_timestamps() and this? Why are we not creating a tombstone file or directory with a predictable name that is guaranteed to be an atomic operation and then storing the timestamp for aging inside?","commit_id":"925ad5e1ea441a7aebaa4ff39fdaf76b56fbb894"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"449f66ca66ea1414d4df4837e4e8ab08084300d4","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                ts_path \u003d \u0027%s/%s\u0027 % (path,"},{"line_number":136,"context_line":"                                     self._get_timestamp_filename())"},{"line_number":137,"context_line":"                try:"},{"line_number":138,"context_line":"                    ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":139,"context_line":"                except error_util.FileAlreadyExistsException:"},{"line_number":140,"context_line":"                    LOG.debug(_(\"Timestamp already exists. Created \""},{"line_number":141,"context_line":"                                \"by another node.\"))"}],"source_content_type":"text/x-python","patch_set":60,"id":"AAAAVn%2F%2FWNw%3D","line":138,"in_reply_to":"AAAAVn%2F%2FXKc%3D","updated":"2014-03-06 17:47:28.000000000","message":"There are a number of issues here:\n1. race between get_timestamps and this call. this may happen if there is more than one nova compute running and they both hit the same code for the the same image on the same data store at the same time. both will get empty sets of timestamps and try and create the timestamp folder. there are two cases here:\ni. they will bot try and create the timestamp folder with exactly the same name - then only one will succeed.\nii. they will create timestamp folder with different times. this will mean that this specific image folder will have more than one timestamp folder. This is ok as we will always do the aging according to the first one that is read. please note that when the image has aged the image folder will be deleted so all of the directories below will be deleted too (including the timestamps). I have tested with 8 nova computes and as of yet have not encountered this race.\n2. Using a tombstone file. When we did the design this was something that we took into account. It really complicates matters. I will try and explain. There may be an edge case where nova-compute1 creates the tombstone directory and then has an exception or a power failure. nova compute 2 will see the file and then either jump over (as it assumes that another process is creating the timestamp) or wait for a specific timeout on the folder. The latter could create the same issue as discussed above. The former could have the timestamp never being created on the backend datastore.  This is the reason why we decided to go for the option of tolerating the race and then treating the fact that there may be more than one timestamp.\nI think that the predictable name has problems that are too costly for something that can be self healing.\nI hope that ths make sense.","commit_id":"925ad5e1ea441a7aebaa4ff39fdaf76b56fbb894"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"f64b9d38f5e095591c7855a34e50b0e20fd0bc2e","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            else:"},{"line_number":145,"context_line":"                # Get the first timestamp (there may be edge cases when"},{"line_number":146,"context_line":"                # there are more)"},{"line_number":147,"context_line":"                ts \u003d timestamps.pop()"},{"line_number":148,"context_line":"                dt \u003d self._get_datetime_from_filename(ts)"},{"line_number":149,"context_line":"                if timeutils.is_older_than(dt, age_seconds):"},{"line_number":150,"context_line":"                    LOG.info(_(\"Image %s is no longer used. \""}],"source_content_type":"text/x-python","patch_set":60,"id":"AAAAVn%2F%2FWMw%3D","line":147,"updated":"2014-03-06 17:50:49.000000000","message":"Yeah... so we get all the timestamps that were created in a race, and then choose a random (maybe) one and compare that (should be the earliest maybe)... this AT LEAST requires a big comment!\n\nI am maybe kinda OK with this behaviour, but I really shouldn\u0027t be since it\u0027s really not that hard to fix.\n\nThe comment above suggest that it may be since using folders \"simplifies things\".\n\nAlso - how much harder will it be to fix later once we have deployers using this?\n\nEither way - we need to be honest about it, and not say \"may be edge cases\". that\u0027s just Ninja-ing a race into the codebase speak :)","commit_id":"925ad5e1ea441a7aebaa4ff39fdaf76b56fbb894"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"6ea27c809c5581207fab10478563ab0ec738ac0b","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            else:"},{"line_number":145,"context_line":"                # Get the first timestamp (there may be edge cases when"},{"line_number":146,"context_line":"                # there are more)"},{"line_number":147,"context_line":"                ts \u003d timestamps.pop()"},{"line_number":148,"context_line":"                dt \u003d self._get_datetime_from_filename(ts)"},{"line_number":149,"context_line":"                if timeutils.is_older_than(dt, age_seconds):"},{"line_number":150,"context_line":"                    LOG.info(_(\"Image %s is no longer used. \""}],"source_content_type":"text/x-python","patch_set":60,"id":"AAAAVn%2F%2FTuo%3D","line":147,"in_reply_to":"AAAAVn%2F%2FUY0%3D","updated":"2014-03-06 22:39:46.000000000","message":"I have added more comments here and have added code that returns the newest timestamp.","commit_id":"925ad5e1ea441a7aebaa4ff39fdaf76b56fbb894"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"ca4e0f718cc875ae7fe9622162524072d9105bcb","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            else:"},{"line_number":145,"context_line":"                # Get the first timestamp (there may be edge cases when"},{"line_number":146,"context_line":"                # there are more)"},{"line_number":147,"context_line":"                ts \u003d timestamps.pop()"},{"line_number":148,"context_line":"                dt \u003d self._get_datetime_from_filename(ts)"},{"line_number":149,"context_line":"                if timeutils.is_older_than(dt, age_seconds):"},{"line_number":150,"context_line":"                    LOG.info(_(\"Image %s is no longer used. \""}],"source_content_type":"text/x-python","patch_set":60,"id":"AAAAVn%2F%2FUY0%3D","line":147,"in_reply_to":"AAAAVn%2F%2FWMw%3D","updated":"2014-03-06 21:20:16.000000000","message":"I would agree we should put more comments as a matter of explanation. But it is more to acknowledge the presence of this race, and how it does not matter to the aging logic.\n\nUsing a single timestamp file with predictable name would have been nice, but it is oddly rather awkward to do using the vSphere API, requiring additional roundtrips to the server, doing HTTP put/get\u0027s that we can do without. And at the end of the day, there is no stopping a slightly laggy compute node from updating the same file \"back in time\" without more checks.\n\nFinally, it should be noted that the time window at which this race involving multiple compute nodes is several orders of magnitude smaller that the image aging interval, which is why it does not look to be all that significant in the grand scheme of things.","commit_id":"925ad5e1ea441a7aebaa4ff39fdaf76b56fbb894"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6e30ee0eda2c2f055bd444a9b970d04d93d69e15","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self._base_folder \u003d base_folder"},{"line_number":58,"context_line":"        self._ds_browser \u003d {}"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def _folder_delete(self, path, dc_ref):"},{"line_number":61,"context_line":"        try:"},{"line_number":62,"context_line":"            ds_util.file_delete(self._session, path, dc_ref)"},{"line_number":63,"context_line":"        except (error_util.CannotDeleteFileException,"}],"source_content_type":"text/x-python","patch_set":62,"id":"AAAAVn%2F%2FPQE%3D","line":60,"updated":"2014-03-07 11:25:30.000000000","message":"I just wrote up a detailed explanation of why the raciness of multiple timestamps is both necessary given the limitations of the vSphere API, and handled safely by this code.\n\nUnfortunately, in doing so I hit upon another problem: spawn() doesn\u0027t take account of an image cache directory potentially disappearing underneath it. If an image cache directory were being deleted while a new instance using it was being created, it would certainly fail and potentially end up in an undesirable state.\n\nI don\u0027t immediately see a way round this without using locks. There is utils.synchronized(), but this is single node. In order to use it, we have to assure ourselves that there is no reasonable architecture in which 2 Nova nodes will be running against the same datastore. If this isn\u0027t the case, as we\u0027ve assumed elsewhere, we\u0027ve got a seriously tricky problem, as we need to do multi-node locking. Ideally this would be done with the vSphere API, but as we\u0027ve been discussing recently this will not be a simple task.","commit_id":"feb8a9438c61cb632a03cf563be26f7c7fde7227"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"dc26eebc71aca6a6c03aa3f8ab357eeb6ca78a8a","unresolved":false,"context_lines":[{"line_number":133,"context_line":"            path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":134,"context_line":"            # Lock to ensure that the spawn will not try and access a image"},{"line_number":135,"context_line":"            # that is currently being deleted on the datastore."},{"line_number":136,"context_line":"            with lockutils.lock(path, lock_file_prefix\u003d\u0027nova-vmware\u0027,"},{"line_number":137,"context_line":"                                external\u003dTrue):"},{"line_number":138,"context_line":"                timestamps \u003d self._get_timestamps(ds_browser, path)"},{"line_number":139,"context_line":"                if not timestamps:"}],"source_content_type":"text/x-python","patch_set":68,"id":"AAAAVn%2F%2FChM%3D","line":136,"updated":"2014-03-10 14:20:00.000000000","message":"We need to clearly document what resource this lock is controlling, and how it is to be correctly used. As written, this is now a lock on *timestamps*. It can be described as:\n\nBefore reading, creating, or using a timestamp, the user must first take out a timestamp lock on the containing image cache directory.\n\nThis will make cache ageing deterministic.","commit_id":"0729ce6f3484a6b4d5ebcbedbce0099bbbeae31e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7bc3b6f7560160e66b5643b252727371bee3bb66","unresolved":false,"context_lines":[{"line_number":133,"context_line":"            path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":134,"context_line":"            # Lock to ensure that the spawn will not try and access a image"},{"line_number":135,"context_line":"            # that is currently being deleted on the datastore."},{"line_number":136,"context_line":"            with lockutils.lock(path, lock_file_prefix\u003d\u0027nova-vmware\u0027,"},{"line_number":137,"context_line":"                                external\u003dTrue):"},{"line_number":138,"context_line":"                timestamps \u003d self._get_timestamps(ds_browser, path)"},{"line_number":139,"context_line":"                if not timestamps:"}],"source_content_type":"text/x-python","patch_set":68,"id":"AAAAVn%2F%2FCbk%3D","line":136,"in_reply_to":"AAAAVn%2F%2FChM%3D","updated":"2014-03-10 14:25:34.000000000","message":"Done","commit_id":"0729ce6f3484a6b4d5ebcbedbce0099bbbeae31e"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"dc26eebc71aca6a6c03aa3f8ab357eeb6ca78a8a","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        # the specific image."},{"line_number":172,"context_line":"        for image in self.used_images:"},{"line_number":173,"context_line":"            path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":174,"context_line":"            self.timestamp_cleanup(dc_info.ref, ds_browser,"},{"line_number":175,"context_line":"                                   datastore[\u0027ref\u0027], datastore[\u0027name\u0027],"},{"line_number":176,"context_line":"                                   path)"},{"line_number":177,"context_line":""}],"source_content_type":"text/x-python","patch_set":68,"id":"AAAAVn%2F%2FCgg%3D","line":174,"updated":"2014-03-10 14:20:00.000000000","message":"We\u0027re deleting timestamps, so by the above rule this requires a timestamp lock on the image cache directory.","commit_id":"0729ce6f3484a6b4d5ebcbedbce0099bbbeae31e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7bc3b6f7560160e66b5643b252727371bee3bb66","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        # the specific image."},{"line_number":172,"context_line":"        for image in self.used_images:"},{"line_number":173,"context_line":"            path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":174,"context_line":"            self.timestamp_cleanup(dc_info.ref, ds_browser,"},{"line_number":175,"context_line":"                                   datastore[\u0027ref\u0027], datastore[\u0027name\u0027],"},{"line_number":176,"context_line":"                                   path)"},{"line_number":177,"context_line":""}],"source_content_type":"text/x-python","patch_set":68,"id":"AAAAVn%2F%2FCbg%3D","line":174,"in_reply_to":"AAAAVn%2F%2FCgg%3D","updated":"2014-03-10 14:25:34.000000000","message":"Done","commit_id":"0729ce6f3484a6b4d5ebcbedbce0099bbbeae31e"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d1b82a4455051434a67930182b61931b141a12c7","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        unused_images \u003d self.originals - self.used_images"},{"line_number":136,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"},{"line_number":137,"context_line":"        for image in unused_images:"},{"line_number":138,"context_line":"            path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":139,"context_line":"            # Lock to ensure that the spawn will not try and access a image"},{"line_number":140,"context_line":"            # that is currently being deleted on the datastore."},{"line_number":141,"context_line":"            with lockutils.lock(path, lock_file_prefix\u003d\u0027nova-vmware-ts\u0027,"}],"source_content_type":"text/x-python","patch_set":70,"id":"AAAAVn%2F%2FBU8%3D","line":138,"updated":"2014-03-10 16:52:03.000000000","message":"You need to lock the same path here as in vmops. Suggest a library function in imagecache to do this, tbh, so you can use it from vmops.py.","commit_id":"a638760bcce0c238a6024022a634e7d80dc4fbb9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"93c8a8041c181626ef7172b6cee5736a01d393c5","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        unused_images \u003d self.originals - self.used_images"},{"line_number":136,"context_line":"        ds_browser \u003d self._get_ds_browser(datastore[\u0027ref\u0027])"},{"line_number":137,"context_line":"        for image in unused_images:"},{"line_number":138,"context_line":"            path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":139,"context_line":"            # Lock to ensure that the spawn will not try and access a image"},{"line_number":140,"context_line":"            # that is currently being deleted on the datastore."},{"line_number":141,"context_line":"            with lockutils.lock(path, lock_file_prefix\u003d\u0027nova-vmware-ts\u0027,"}],"source_content_type":"text/x-python","patch_set":70,"id":"AAAAVn%2F%2FBOM%3D","line":138,"in_reply_to":"AAAAVn%2F%2FBU8%3D","updated":"2014-03-10 17:06:32.000000000","message":"This is the same path as in vmops\nNot sure I understand your point here","commit_id":"a638760bcce0c238a6024022a634e7d80dc4fbb9"},{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"daea5288890a754b7bfe3c1dbfcce80bdbf4b3f2","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        for image in unused_images:"},{"line_number":138,"context_line":"            path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":139,"context_line":"            # Lock to ensure that the spawn will not try and access a image"},{"line_number":140,"context_line":"            # that is currently being deleted on the datastore."},{"line_number":141,"context_line":"            with lockutils.lock(path, lock_file_prefix\u003d\u0027nova-vmware-ts\u0027,"},{"line_number":142,"context_line":"                                external\u003dTrue):"},{"line_number":143,"context_line":"                timestamps \u003d self._get_timestamps(ds_browser, path)"}],"source_content_type":"text/x-python","patch_set":70,"id":"AAAAVn%2F%2FAq8%3D","line":140,"updated":"2014-03-10 18:12:41.000000000","message":"This is only host-local lock, though ... not everything that could be acting on the datastore, right?","commit_id":"a638760bcce0c238a6024022a634e7d80dc4fbb9"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b50ceaadfcf5440544a85030418f75c385badaf6","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        # A list of files in ds_path will be returned - we need to get"},{"line_number":94,"context_line":"        # the timestamp file if it exists."},{"line_number":95,"context_line":"        # There may be edge cases when there is more than one timestamp"},{"line_number":96,"context_line":"        # file. In this case all timestamp files will be returned."},{"line_number":97,"context_line":"        files \u003d ds_util.get_sub_folders(self._session, ds_browser, ds_path)"},{"line_number":98,"context_line":"        timestamps \u003d set()"},{"line_number":99,"context_line":"        if files:"}],"source_content_type":"text/x-python","patch_set":72,"id":"AAAAVn%2F%2B4Jk%3D","line":96,"updated":"2014-03-11 16:01:55.000000000","message":"Not true anymore as well... we can simplify this method.","commit_id":"5c4a9d5e13c27c8394e34fbde1eb0e48c464785d"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d657c112b5fe2279de1483409f4426504536ba71","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        # A list of files in ds_path will be returned - we need to get"},{"line_number":94,"context_line":"        # the timestamp file if it exists."},{"line_number":95,"context_line":"        # There may be edge cases when there is more than one timestamp"},{"line_number":96,"context_line":"        # file. In this case all timestamp files will be returned."},{"line_number":97,"context_line":"        files \u003d ds_util.get_sub_folders(self._session, ds_browser, ds_path)"},{"line_number":98,"context_line":"        timestamps \u003d set()"},{"line_number":99,"context_line":"        if files:"}],"source_content_type":"text/x-python","patch_set":72,"id":"AAAAVn%2F%2B3uU%3D","line":96,"in_reply_to":"AAAAVn%2F%2B4Jk%3D","updated":"2014-03-11 17:02:24.000000000","message":"Done","commit_id":"5c4a9d5e13c27c8394e34fbde1eb0e48c464785d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b50ceaadfcf5440544a85030418f75c385badaf6","unresolved":false,"context_lines":[{"line_number":151,"context_line":"                    try:"},{"line_number":152,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":153,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":154,"context_line":"                        LOG.debug(_(\"Timestamp already exists. Created \""},{"line_number":155,"context_line":"                                    \"by another node.\"))"},{"line_number":156,"context_line":"                    LOG.info(_(\"Image %s is no longer used by this node. \""},{"line_number":157,"context_line":"                               \"Pending deletion!\"), image)"}],"source_content_type":"text/x-python","patch_set":72,"id":"AAAAVn%2F%2B4Wo%3D","line":154,"updated":"2014-03-11 16:01:55.000000000","message":"This is not possible anymore as well... or at least the debug message is wrong.","commit_id":"5c4a9d5e13c27c8394e34fbde1eb0e48c464785d"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d657c112b5fe2279de1483409f4426504536ba71","unresolved":false,"context_lines":[{"line_number":151,"context_line":"                    try:"},{"line_number":152,"context_line":"                        ds_util.mkdir(self._session, ts_path, dc_info.ref)"},{"line_number":153,"context_line":"                    except error_util.FileAlreadyExistsException:"},{"line_number":154,"context_line":"                        LOG.debug(_(\"Timestamp already exists. Created \""},{"line_number":155,"context_line":"                                    \"by another node.\"))"},{"line_number":156,"context_line":"                    LOG.info(_(\"Image %s is no longer used by this node. \""},{"line_number":157,"context_line":"                               \"Pending deletion!\"), image)"}],"source_content_type":"text/x-python","patch_set":72,"id":"AAAAVn%2F%2B3sg%3D","line":154,"in_reply_to":"AAAAVn%2F%2B4Wo%3D","updated":"2014-03-11 17:02:24.000000000","message":"Done","commit_id":"5c4a9d5e13c27c8394e34fbde1eb0e48c464785d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b50ceaadfcf5440544a85030418f75c385badaf6","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                    # In this case we will sort the timestamps and select the"},{"line_number":167,"context_line":"                    # newest timestamp."},{"line_number":168,"context_line":"                    timestamps \u003d sorted(timestamps)"},{"line_number":169,"context_line":"                    ts \u003d timestamps.pop()"},{"line_number":170,"context_line":"                    dt \u003d self._get_datetime_from_filename(ts)"},{"line_number":171,"context_line":"                    if timeutils.is_older_than(dt, age_seconds):"},{"line_number":172,"context_line":"                        LOG.info(_(\"Image %s is no longer used. \""}],"source_content_type":"text/x-python","patch_set":72,"id":"AAAAVn%2F%2B39I%3D","line":169,"updated":"2014-03-11 16:01:55.000000000","message":"The comment and the above two lines are not needed anymore.","commit_id":"5c4a9d5e13c27c8394e34fbde1eb0e48c464785d"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d657c112b5fe2279de1483409f4426504536ba71","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                    # In this case we will sort the timestamps and select the"},{"line_number":167,"context_line":"                    # newest timestamp."},{"line_number":168,"context_line":"                    timestamps \u003d sorted(timestamps)"},{"line_number":169,"context_line":"                    ts \u003d timestamps.pop()"},{"line_number":170,"context_line":"                    dt \u003d self._get_datetime_from_filename(ts)"},{"line_number":171,"context_line":"                    if timeutils.is_older_than(dt, age_seconds):"},{"line_number":172,"context_line":"                        LOG.info(_(\"Image %s is no longer used. \""}],"source_content_type":"text/x-python","patch_set":72,"id":"AAAAVn%2F%2B3sY%3D","line":169,"in_reply_to":"AAAAVn%2F%2B39I%3D","updated":"2014-03-11 17:02:24.000000000","message":"Done","commit_id":"5c4a9d5e13c27c8394e34fbde1eb0e48c464785d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b50ceaadfcf5440544a85030418f75c385badaf6","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        # exists then this must be deleted. The timestamp would have been"},{"line_number":179,"context_line":"        # created by another node that no longer has instances running for"},{"line_number":180,"context_line":"        # the specific image."},{"line_number":181,"context_line":"        for image in self.used_images:"},{"line_number":182,"context_line":"            path \u003d self.timestamp_folder_get(ds_path, image)"},{"line_number":183,"context_line":"            with lockutils.lock(path, lock_file_prefix\u003d\u0027nova-vmware-ts\u0027,"},{"line_number":184,"context_line":"                                external\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":72,"id":"AAAAVn%2F%2B4FE%3D","line":181,"updated":"2014-03-11 16:01:55.000000000","message":"This is also not needed anymore I believe...","commit_id":"5c4a9d5e13c27c8394e34fbde1eb0e48c464785d"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d657c112b5fe2279de1483409f4426504536ba71","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        # exists then this must be deleted. The timestamp would have been"},{"line_number":179,"context_line":"        # created by another node that no longer has instances running for"},{"line_number":180,"context_line":"        # the specific image."},{"line_number":181,"context_line":"        for image in self.used_images:"},{"line_number":182,"context_line":"            path \u003d self.timestamp_folder_get(ds_path, image)"},{"line_number":183,"context_line":"            with lockutils.lock(path, lock_file_prefix\u003d\u0027nova-vmware-ts\u0027,"},{"line_number":184,"context_line":"                                external\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":72,"id":"AAAAVn%2F%2B3sM%3D","line":181,"in_reply_to":"AAAAVn%2F%2B4FE%3D","updated":"2014-03-11 17:02:24.000000000","message":"Done","commit_id":"5c4a9d5e13c27c8394e34fbde1eb0e48c464785d"}],"nova/virt/vmwareapi/vm_util.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":1154,"context_line":"    data_stores \u003d session._call_method(vim_util,"},{"line_number":1155,"context_line":"            \"get_properties_for_a_collection_of_objects\","},{"line_number":1156,"context_line":"            \"Datastore\", data_store_mors,"},{"line_number":1157,"context_line":"            [\"summary.type\", \"summary.name\", \"summary.accessible\"])"},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"    allowed \u003d []"},{"line_number":1160,"context_line":"    while data_stores:"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FrXY%3D","line":1157,"updated":"2014-03-04 19:09:44.000000000","message":"This can be done in a single API call using an appropriate Traversal and RetrievePropertiesEx","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":1154,"context_line":"    data_stores \u003d session._call_method(vim_util,"},{"line_number":1155,"context_line":"            \"get_properties_for_a_collection_of_objects\","},{"line_number":1156,"context_line":"            \"Datastore\", data_store_mors,"},{"line_number":1157,"context_line":"            [\"summary.type\", \"summary.name\", \"summary.accessible\"])"},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"    allowed \u003d []"},{"line_number":1160,"context_line":"    while data_stores:"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2Fn6A%3D","line":1157,"in_reply_to":"AAAAVn%2F%2FrXY%3D","updated":"2014-03-05 02:06:38.000000000","message":"Good suggestion. At the moment there is a patch doing just that https://review.openstack.org/#/c/76317/\nonce that lands then i\u0027ll update here.\nIt is a nice improvement and i\u0027\u0027l make a note here.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"83520c95fa8d613fe12d8eaf924d437aa56c06a0","unresolved":false,"context_lines":[{"line_number":1167,"context_line":"        data_stores \u003d session._call_method(vim_util,"},{"line_number":1168,"context_line":"                                           \"continue_to_get_objects\","},{"line_number":1169,"context_line":"                                           token)"},{"line_number":1170,"context_line":"    return allowed"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_vmdk_backed_disk_uuid(hardware_devices, volume_uuid):"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FrNI%3D","line":1170,"updated":"2014-03-04 18:23:47.000000000","message":"I don\u0027t see a test for this method. In general, it would be helpful to have new utility methods broken out to their own patch with tests ahead of the one that uses it, IMHO. We can approve them together, but it\u0027s hard for me to comb through all of this and make sure that everything is getting test coverage when so much is changing. AFAICT, the above (at least) could stand on its own.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":1167,"context_line":"        data_stores \u003d session._call_method(vim_util,"},{"line_number":1168,"context_line":"                                           \"continue_to_get_objects\","},{"line_number":1169,"context_line":"                                           token)"},{"line_number":1170,"context_line":"    return allowed"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_vmdk_backed_disk_uuid(hardware_devices, volume_uuid):"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2Fnb8%3D","line":1170,"in_reply_to":"AAAAVn%2F%2FrNI%3D","updated":"2014-03-05 02:06:38.000000000","message":"yes, you are correct. i should have done that. this code is tested in the aging methods that we have. i should have added a specific test for this. sorry my bad. i will make a note to add one","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"}],"nova/virt/vmwareapi/vmops.py":[{"author":{"_account_id":7040,"name":"GuoHui LIu","email":"liuguohui@gmail.com","username":"guohliu"},"change_message_id":"71deceb3d4bd0ba4c959da42cf6a5b66ed1bfaab","unresolved":false,"context_lines":[{"line_number":1689,"context_line":"                for image in unused_images:"},{"line_number":1690,"context_line":"                    # Delete the directory"},{"line_number":1691,"context_line":"                    path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":1692,"context_line":"                    LOG.info(\"Image %s is no longer used\", path)"},{"line_number":1693,"context_line":"                    vmdk_delete_task \u003d self._session._call_method("},{"line_number":1694,"context_line":"                                self._session._get_vim(),"},{"line_number":1695,"context_line":"                                \"DeleteDatastoreFile_Task\","}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2FFDI%3D","line":1692,"updated":"2013-11-14 17:33:28.000000000","message":"i18n issue, missing _ for message.","commit_id":"e675acc239166ecd902bc3c7d84ee8cc50c6f958"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"9b6c3993869eb2edde467fcc5ecaa8e2fd11331c","unresolved":false,"context_lines":[{"line_number":1689,"context_line":"                for image in unused_images:"},{"line_number":1690,"context_line":"                    # Delete the directory"},{"line_number":1691,"context_line":"                    path \u003d \u0027%s/%s\u0027 % (ds_path, image)"},{"line_number":1692,"context_line":"                    LOG.info(\"Image %s is no longer used\", path)"},{"line_number":1693,"context_line":"                    vmdk_delete_task \u003d self._session._call_method("},{"line_number":1694,"context_line":"                                self._session._get_vim(),"},{"line_number":1695,"context_line":"                                \"DeleteDatastoreFile_Task\","}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2FFBM%3D","line":1692,"in_reply_to":"AAAATn%2F%2FFDI%3D","updated":"2013-11-14 17:45:27.000000000","message":"Good catch! Other than saying the dog ta my homework, I can say that it is WIP :)","commit_id":"e675acc239166ecd902bc3c7d84ee8cc50c6f958"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"815331487e6d1d026e34c73b91be508a53d7ea07","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        folder_path \u003d ds_util.build_datastore_path(ds_name, name)"},{"line_number":140,"context_line":"        if not ds_util.path_exists(self._session, ds_browser, folder_path):"},{"line_number":141,"context_line":"            dc_info \u003d self.get_datacenter_ref_and_name(ds_ref)"},{"line_number":142,"context_line":"            ds_util.mkdir(self._session,"},{"line_number":143,"context_line":"                          ds_util.build_datastore_path(ds_name, name),"},{"line_number":144,"context_line":"                          dc_info.ref)"},{"line_number":145,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAUH%2F9oSw%3D","line":142,"updated":"2014-02-05 14:48:25.000000000","message":"Should ds_util.mkdir() be wrapped in try:  .... except error_util.get_fault_class(\u0027AlreadyExists\u0027):  to avoid a possible race condition (similar to the patch you had for the race condition around creating files)? \n\nAlso, it might be worth while to not check if it exists first and just try to create the dir as it would reduce one extra call to vc. Probably doesn\u0027t matter though.","commit_id":"4ac190c02f3c3da482fc68a28c6cdf844cdee5d7"},{"author":{"_account_id":7400,"name":"Tracy Jones","email":"tjones@vmware.com","username":"tracyajones"},"change_message_id":"79b4c396f7a9148ffb3b0b4faded19a5c032ff62","unresolved":false,"context_lines":[{"line_number":149,"context_line":"            # on the datastore. Currently the only way of detecting that"},{"line_number":150,"context_line":"            # the folder exists is to catch the general exception."},{"line_number":151,"context_line":"            pass"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    def _extend_virtual_disk(self, instance, requested_size, name,"},{"line_number":154,"context_line":"                             datacenter):"},{"line_number":155,"context_line":"        service_content \u003d self._session._get_vim().get_service_content()"}],"source_content_type":"text/x-python","patch_set":33,"id":"AAAAUH%2F9Uzo%3D","line":152,"updated":"2014-02-07 18:17:35.000000000","message":"can this move to ds_util as well ?","commit_id":"5bc966d57b1c56303b65e0ba824d1e86153f4705"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        return lst_vm_names"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _extend_virtual_disk(self, instance, requested_size, name,"},{"line_number":139,"context_line":"                             datacenter):"},{"line_number":140,"context_line":"        service_content \u003d self._session._get_vim().get_service_content()"},{"line_number":141,"context_line":"        LOG.debug(_(\"Extending root virtual disk to %s\"), requested_size)"},{"line_number":142,"context_line":"        vmdk_extend_task \u003d self._session._call_method("}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FrsQ%3D","line":139,"updated":"2014-03-04 19:09:44.000000000","message":"This is an unrelated change. I would have preferred to see this in a separate commit.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        return lst_vm_names"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _extend_virtual_disk(self, instance, requested_size, name,"},{"line_number":139,"context_line":"                             datacenter):"},{"line_number":140,"context_line":"        service_content \u003d self._session._get_vim().get_service_content()"},{"line_number":141,"context_line":"        LOG.debug(_(\"Extending root virtual disk to %s\"), requested_size)"},{"line_number":142,"context_line":"        vmdk_extend_task \u003d self._session._call_method("}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FqjE%3D","line":139,"in_reply_to":"AAAAVn%2F%2FrsQ%3D","updated":"2014-03-05 02:06:38.000000000","message":"Done","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"70bc81b9fe32f35f3952cf84bb9e928d073dfd43","unresolved":false,"context_lines":[{"line_number":1576,"context_line":"            # just means the folder was prepped by another process."},{"line_number":1577,"context_line":"            pass"},{"line_number":1578,"context_line":""},{"line_number":1579,"context_line":"    def _check_if_folder_file_exists(self, ds_browser, ds_ref, ds_name,"},{"line_number":1580,"context_line":"                                     folder_name, file_name):"},{"line_number":1581,"context_line":"        # Ensure that the cache folder exists"},{"line_number":1582,"context_line":"        self._create_folder_if_missing(ds_name, ds_ref, self._base_folder)"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FrtU%3D","line":1579,"updated":"2014-03-04 19:09:44.000000000","message":"This is an unrelated change. I would have preferred to see this in a separate commit.","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1a6a68a3c2469e5e6ba52c844f8ed2cec03d575","unresolved":false,"context_lines":[{"line_number":1576,"context_line":"            # just means the folder was prepped by another process."},{"line_number":1577,"context_line":"            pass"},{"line_number":1578,"context_line":""},{"line_number":1579,"context_line":"    def _check_if_folder_file_exists(self, ds_browser, ds_ref, ds_name,"},{"line_number":1580,"context_line":"                                     folder_name, file_name):"},{"line_number":1581,"context_line":"        # Ensure that the cache folder exists"},{"line_number":1582,"context_line":"        self._create_folder_if_missing(ds_name, ds_ref, self._base_folder)"}],"source_content_type":"text/x-python","patch_set":56,"id":"AAAAVn%2F%2FqfI%3D","line":1579,"in_reply_to":"AAAAVn%2F%2FrtU%3D","updated":"2014-03-05 02:06:38.000000000","message":"that is related to the change. it also save us a backend call. the ds_browser is needed on line 597. this also save multiple calls","commit_id":"bf403c677d423f1b57212db775f9c03b25eff429"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"f64b9d38f5e095591c7855a34e50b0e20fd0bc2e","unresolved":false,"context_lines":[{"line_number":541,"context_line":"                                                    root_gb)"},{"line_number":542,"context_line":"                root_vmdk_path \u003d ds_util.build_datastore_path(data_store_name,"},{"line_number":543,"context_line":"                                                              root_vmdk_name)"},{"line_number":544,"context_line":"                if not self._check_if_folder_file_exists(ds_browser,"},{"line_number":545,"context_line":"                                        data_store_ref, data_store_name,"},{"line_number":546,"context_line":"                                        upload_folder,"},{"line_number":547,"context_line":"                                        upload_name + \".%s.vmdk\" % root_gb):"}],"source_content_type":"text/x-python","patch_set":60,"id":"AAAAVn%2F%2FXHA%3D","line":544,"updated":"2014-03-06 17:50:49.000000000","message":"OK so this is going to conflict with the ISO patch... sigh","commit_id":"925ad5e1ea441a7aebaa4ff39fdaf76b56fbb894"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"6ea27c809c5581207fab10478563ab0ec738ac0b","unresolved":false,"context_lines":[{"line_number":541,"context_line":"                                                    root_gb)"},{"line_number":542,"context_line":"                root_vmdk_path \u003d ds_util.build_datastore_path(data_store_name,"},{"line_number":543,"context_line":"                                                              root_vmdk_name)"},{"line_number":544,"context_line":"                if not self._check_if_folder_file_exists(ds_browser,"},{"line_number":545,"context_line":"                                        data_store_ref, data_store_name,"},{"line_number":546,"context_line":"                                        upload_folder,"},{"line_number":547,"context_line":"                                        upload_name + \".%s.vmdk\" % root_gb):"}],"source_content_type":"text/x-python","patch_set":60,"id":"AAAAVn%2F%2FUTU%3D","line":544,"in_reply_to":"AAAAVn%2F%2FXHA%3D","updated":"2014-03-06 22:39:46.000000000","message":"Done","commit_id":"925ad5e1ea441a7aebaa4ff39fdaf76b56fbb894"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b79a11854135faeb3ba85c09fd7a31c3744fba8c","unresolved":false,"context_lines":[{"line_number":460,"context_line":"            # Lock to ensure that the spawn will not try and access a image"},{"line_number":461,"context_line":"            # that is currently being deleted on the datastore."},{"line_number":462,"context_line":"            with lockutils.lock(ts_path, lock_file_prefix\u003d\u0027nova-vmware\u0027,"},{"line_number":463,"context_line":"                                external\u003dTrue):"},{"line_number":464,"context_line":"                if CONF.remove_unused_base_images:"},{"line_number":465,"context_line":"                    self._imagecache.timestamp_cleanup(dc_info.ref,"},{"line_number":466,"context_line":"                            ds_browser, data_store_ref, data_store_name,"}],"source_content_type":"text/x-python","patch_set":66,"id":"AAAAVn%2F%2FDrM%3D","line":463,"updated":"2014-03-10 10:41:22.000000000","message":"This would serialise all spawns from a single image, which isn\u0027t desirable.","commit_id":"db9730794031a6ff83b72fcffd5cb9372492ac64"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"dc26eebc71aca6a6c03aa3f8ab357eeb6ca78a8a","unresolved":false,"context_lines":[{"line_number":463,"context_line":"            with lockutils.lock(ts_path, lock_file_prefix\u003d\u0027nova-vmware\u0027,"},{"line_number":464,"context_line":"                                external\u003dTrue):"},{"line_number":465,"context_line":"                if CONF.remove_unused_base_images:"},{"line_number":466,"context_line":"                    self._imagecache.timestamp_cleanup(dc_info.ref,"},{"line_number":467,"context_line":"                            ds_browser, data_store_ref, data_store_name,"},{"line_number":468,"context_line":"                            ts_path)"},{"line_number":469,"context_line":""}],"source_content_type":"text/x-python","patch_set":68,"id":"AAAAVn%2F%2FCfE%3D","line":466,"updated":"2014-03-10 14:20:00.000000000","message":"By our timestamp lock rule, we\u0027re now saying that we can guarantee the image cache directory won\u0027t be deleted under us for at least the next \u003cconfig time\u003e. Although this isn\u0027t strictly 100% safe, it\u0027s a pragmatic guarantee IMHO.","commit_id":"0729ce6f3484a6b4d5ebcbedbce0099bbbeae31e"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"dc26eebc71aca6a6c03aa3f8ab357eeb6ca78a8a","unresolved":false,"context_lines":[{"line_number":469,"context_line":""},{"line_number":470,"context_line":"                # Check if the image exists in the datastore cache. If not the"},{"line_number":471,"context_line":"                # image will be uploaded and cached."},{"line_number":472,"context_line":"                image_cached \u003d self._check_if_folder_file_exists(ds_browser,"},{"line_number":473,"context_line":"                        data_store_ref, data_store_name, upload_folder,"},{"line_number":474,"context_line":"                        upload_file_name)"},{"line_number":475,"context_line":""}],"source_content_type":"text/x-python","patch_set":68,"id":"AAAAVn%2F%2FCgA%3D","line":472,"updated":"2014-03-10 14:20:00.000000000","message":"By our timestamp lock rule, this doesn\u0027t need to be locked.","commit_id":"0729ce6f3484a6b4d5ebcbedbce0099bbbeae31e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7bc3b6f7560160e66b5643b252727371bee3bb66","unresolved":false,"context_lines":[{"line_number":469,"context_line":""},{"line_number":470,"context_line":"                # Check if the image exists in the datastore cache. If not the"},{"line_number":471,"context_line":"                # image will be uploaded and cached."},{"line_number":472,"context_line":"                image_cached \u003d self._check_if_folder_file_exists(ds_browser,"},{"line_number":473,"context_line":"                        data_store_ref, data_store_name, upload_folder,"},{"line_number":474,"context_line":"                        upload_file_name)"},{"line_number":475,"context_line":""}],"source_content_type":"text/x-python","patch_set":68,"id":"AAAAVn%2F%2FCbo%3D","line":472,"in_reply_to":"AAAAVn%2F%2FCgA%3D","updated":"2014-03-10 14:25:34.000000000","message":"Done","commit_id":"0729ce6f3484a6b4d5ebcbedbce0099bbbeae31e"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b50ceaadfcf5440544a85030418f75c385badaf6","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                files \u003d [name.replace(\".vmdk\", \"-flat.vmdk\"), name]"},{"line_number":165,"context_line":"                for file in files:"},{"line_number":166,"context_line":"                    self._delete_datastore_file(instance, file,"},{"line_number":167,"context_line":"                                                dc_ref)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        LOG.debug(_(\"Extended root virtual disk\"))"},{"line_number":170,"context_line":""}],"source_content_type":"text/x-python","patch_set":72,"id":"AAAAVn%2F%2B4Co%3D","line":167,"updated":"2014-03-11 16:01:55.000000000","message":"this is just whitespace??","commit_id":"5c4a9d5e13c27c8394e34fbde1eb0e48c464785d"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d657c112b5fe2279de1483409f4426504536ba71","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                files \u003d [name.replace(\".vmdk\", \"-flat.vmdk\"), name]"},{"line_number":165,"context_line":"                for file in files:"},{"line_number":166,"context_line":"                    self._delete_datastore_file(instance, file,"},{"line_number":167,"context_line":"                                                dc_ref)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        LOG.debug(_(\"Extended root virtual disk\"))"},{"line_number":170,"context_line":""}],"source_content_type":"text/x-python","patch_set":72,"id":"AAAAVn%2F%2B3rg%3D","line":167,"in_reply_to":"AAAAVn%2F%2B4Co%3D","updated":"2014-03-11 17:02:24.000000000","message":"Done","commit_id":"5c4a9d5e13c27c8394e34fbde1eb0e48c464785d"}]}