[PATCH 0/3] Issue #543
Crístian Viana (3): bugfix: Fix regexp in "kimchi.utils.get_next_clone_name" Support different "tags" when generating a new name in "get_next_clone_name" issue #543: Generate unique vol names when downloading files with same name src/kimchi/model/storagevolumes.py | 12 ++++++++++-- src/kimchi/utils.py | 17 ++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-) -- 1.9.3
The regular expression used in the function "kimchi.utils.get_next_clone_name" has an extra '-' which isn't needed. The following commands, executed from an interactive Python shell, demonstrate the bug:
from kimchi.utils import get_next_clone_name get_next_clone_name(['foo'], 'index', '.html') u'index-clone-1.html' get_next_clone_name(['foo', 'index-clone-1.html'], 'index', '.html') u'index-clone-1.html'
Even though the name 'index-clone-1.html' is provided as an existing name, the function still returns a duplicate value. Fix the regular expression by removing an unnecessary character. The following commands demonstrate the bug fix:
from kimchi.utils import get_next_clone_name get_next_clone_name(['foo'], 'index', '.html') u'index-clone-1.html' get_next_clone_name(['foo', 'index-clone-1.html'], 'index', '.html') u'index-clone-2.html' get_next_clone_name(['foo', 'index-clone-1.html', 'index-clone-2.html'], 'index', '.html') u'index-clone-3.html'
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index 989e370..3af701c 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -335,7 +335,7 @@ def get_next_clone_name(all_names, basename, name_suffix=''): re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) if name_suffix != '': - re_expr = u'%s-%s' % (re_expr, name_suffix) + re_expr = u'%s%s' % (re_expr, name_suffix) max_num = 0 re_compiled = re.compile(re_expr) -- 1.9.3
Reviewed-by: Royce Lv<lvroyce@linux.vnet.ibm.com> On 2014年12月10日 11:17, Crístian Viana wrote:
The regular expression used in the function "kimchi.utils.get_next_clone_name" has an extra '-' which isn't needed.
The following commands, executed from an interactive Python shell, demonstrate the bug:
from kimchi.utils import get_next_clone_name get_next_clone_name(['foo'], 'index', '.html') u'index-clone-1.html' get_next_clone_name(['foo', 'index-clone-1.html'], 'index', '.html') u'index-clone-1.html'
Even though the name 'index-clone-1.html' is provided as an existing name, the function still returns a duplicate value.
Fix the regular expression by removing an unnecessary character.
The following commands demonstrate the bug fix:
from kimchi.utils import get_next_clone_name get_next_clone_name(['foo'], 'index', '.html') u'index-clone-1.html' get_next_clone_name(['foo', 'index-clone-1.html'], 'index', '.html') u'index-clone-2.html' get_next_clone_name(['foo', 'index-clone-1.html', 'index-clone-2.html'], 'index', '.html') u'index-clone-3.html'
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index 989e370..3af701c 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -335,7 +335,7 @@ def get_next_clone_name(all_names, basename, name_suffix=''):
re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) if name_suffix != '': - re_expr = u'%s-%s' % (re_expr, name_suffix) + re_expr = u'%s%s' % (re_expr, name_suffix)
max_num = 0 re_compiled = re.compile(re_expr)
Reviewed-by: Royce Lv<lvroyce@linux.vnet.ibm.com> On 2014年12月10日 11:17, Crístian Viana wrote:
The regular expression used in the function "kimchi.utils.get_next_clone_name" has an extra '-' which isn't needed.
The following commands, executed from an interactive Python shell, demonstrate the bug:
from kimchi.utils import get_next_clone_name get_next_clone_name(['foo'], 'index', '.html') u'index-clone-1.html' get_next_clone_name(['foo', 'index-clone-1.html'], 'index', '.html') u'index-clone-1.html'
Even though the name 'index-clone-1.html' is provided as an existing name, the function still returns a duplicate value.
Fix the regular expression by removing an unnecessary character.
The following commands demonstrate the bug fix:
from kimchi.utils import get_next_clone_name get_next_clone_name(['foo'], 'index', '.html') u'index-clone-1.html' get_next_clone_name(['foo', 'index-clone-1.html'], 'index', '.html') u'index-clone-2.html' get_next_clone_name(['foo', 'index-clone-1.html', 'index-clone-2.html'], 'index', '.html') u'index-clone-3.html'
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index 989e370..3af701c 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -335,7 +335,7 @@ def get_next_clone_name(all_names, basename, name_suffix=''):
re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) if name_suffix != '': - re_expr = u'%s-%s' % (re_expr, name_suffix) + re_expr = u'%s%s' % (re_expr, name_suffix)
max_num = 0 re_compiled = re.compile(re_expr)
The current implementation of "kimchi.utils.get_next_clone_name" always appends the tag "-clone" in its return values. That may not be suitable for other cases when we want a new resource name without that tag. Add an optional parameter in "get_next_clone_name" so it supports different tags. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/utils.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index 3af701c..bac07df 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -312,10 +312,11 @@ def validate_repo_url(url): raise InvalidParameter("KCHREPOS0002E") -def get_next_clone_name(all_names, basename, name_suffix=''): +def get_next_clone_name(all_names, basename, name_suffix='', + clone_tag='-clone'): """Find the next available name for a cloned resource. - If any resource named "<basename>-clone-<number><name_suffix>" is found + If any resource named "<basename><tag>-<number><name_suffix>" is found in "all_names", use the maximum "number" + 1; else, use 1. Arguments: @@ -325,15 +326,17 @@ def get_next_clone_name(all_names, basename, name_suffix=''): basename -- The name of the original resource. name_suffix -- The resource name suffix (optional). This parameter exist so that a resource named "foo.img" gets the name - "foo-clone-1.img" instead of "foo.img-clone-1". If this parameter + "foo<tag>-1.img" instead of "foo.img<tag>-1". If this parameter is used, the suffix should not be present in "basename". + clone_tag -- The tag which will be appended to the new name (optional). + Default value: "-clone" Return: - A UTF-8 string in the format "<basename>-clone-<number><name_suffix>". + A UTF-8 string in the format "<basename><tag>-<number><name_suffix>". """ re_group_num = 'num' - re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) + re_expr = u'%s%s-(?P<%s>\d+)' % (basename, clone_tag, re_group_num) if name_suffix != '': re_expr = u'%s%s' % (re_expr, name_suffix) @@ -346,7 +349,7 @@ def get_next_clone_name(all_names, basename, name_suffix=''): max_num = max(max_num, int(match.group(re_group_num))) # increments the maximum "clone number" found - new_name = u'%s-clone-%d' % (basename, max_num + 1) + new_name = u'%s%s-%d' % (basename, clone_tag, max_num + 1) if name_suffix != '': new_name = new_name + name_suffix -- 1.9.3
Reviewed-by: Royce Lv<lvroyce@linux.vnet.ibm.com> On 2014年12月10日 11:17, Crístian Viana wrote:
The current implementation of "kimchi.utils.get_next_clone_name" always appends the tag "-clone" in its return values. That may not be suitable for other cases when we want a new resource name without that tag.
Add an optional parameter in "get_next_clone_name" so it supports different tags.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/utils.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index 3af701c..bac07df 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -312,10 +312,11 @@ def validate_repo_url(url): raise InvalidParameter("KCHREPOS0002E")
-def get_next_clone_name(all_names, basename, name_suffix=''): +def get_next_clone_name(all_names, basename, name_suffix='', + clone_tag='-clone'): """Find the next available name for a cloned resource.
- If any resource named "<basename>-clone-<number><name_suffix>" is found + If any resource named "<basename><tag>-<number><name_suffix>" is found in "all_names", use the maximum "number" + 1; else, use 1.
Arguments: @@ -325,15 +326,17 @@ def get_next_clone_name(all_names, basename, name_suffix=''): basename -- The name of the original resource. name_suffix -- The resource name suffix (optional). This parameter exist so that a resource named "foo.img" gets the name - "foo-clone-1.img" instead of "foo.img-clone-1". If this parameter + "foo<tag>-1.img" instead of "foo.img<tag>-1". If this parameter is used, the suffix should not be present in "basename". + clone_tag -- The tag which will be appended to the new name (optional). + Default value: "-clone"
Return: - A UTF-8 string in the format "<basename>-clone-<number><name_suffix>". + A UTF-8 string in the format "<basename><tag>-<number><name_suffix>". """ re_group_num = 'num'
- re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) + re_expr = u'%s%s-(?P<%s>\d+)' % (basename, clone_tag, re_group_num) if name_suffix != '': re_expr = u'%s%s' % (re_expr, name_suffix)
@@ -346,7 +349,7 @@ def get_next_clone_name(all_names, basename, name_suffix=''): max_num = max(max_num, int(match.group(re_group_num)))
# increments the maximum "clone number" found - new_name = u'%s-clone-%d' % (basename, max_num + 1) + new_name = u'%s%s-%d' % (basename, clone_tag, max_num + 1) if name_suffix != '': new_name = new_name + name_suffix
If the user downloads multiple remote files with the same name, only the first one will be completed; the following attempts will fail because a volume with the name already exists. The following commands demonstrate the issue: $ kimchi_rest /storagepools/default/storagevolumes | grep name "name":"60f46fdc-8ecc-4601-90d6-b3b44d35c337-0.img", "name":"bdd22e7c-7794-4bdf-8d2b-0089242c8b16-0.img", "name":"c16a7e02-6dcf-48c5-83de-1bb2d8a110a8-0.img", $ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"2", "target_uri":"/storagepools/default/storagevolumes/index.html" } $ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "reason":"KCHVOL0001E: Storage volume index.html already exists", "code":"400 Bad Request", "call_stack":"Traceback (most recent call last):\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, in respond\n response.body = self.handler()\n File \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 188, in __call__\n self.body = self.oldhandler(*args, **kwargs)\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, in __call__\n return self.callable(*self.args, **self.kwargs)\n File \"/home/vianac/LTC/kimchi/src/kimchi/control/base.py\", line 331, in index\n raise cherrypy.HTTPError(400, e.message)\nHTTPError: (400, u'KCHVOL0001E: Storage volume index.html already exists')\n" Detect whether the file to be downloaded will have a name collision with the existing volumes and generate a different name in that case. The following commands demonstrate the bug fix: $ kimchi_rest /storagepools/default/storagevolumes | grep name "name":"60f46fdc-8ecc-4601-90d6-b3b44d35c337-0.img", "name":"bdd22e7c-7794-4bdf-8d2b-0089242c8b16-0.img", "name":"c16a7e02-6dcf-48c5-83de-1bb2d8a110a8-0.img", $ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"1", "target_uri":"/storagepools/default/storagevolumes/index.html" } $ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"2", "target_uri":"/storagepools/default/storagevolumes/index-1.html" } $ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"3", "target_uri":"/storagepools/default/storagevolumes/index-2.html" Fix issue #543 ("Download a volume with same basename will result error"). Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/model/storagevolumes.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 406b38b..0b8bc15 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -76,6 +76,8 @@ class StorageVolumesModel(object): except: raise InvalidParameter('KCHVOL0022E', {'url': url}) + all_vol_names = self.get_list(pool_name) + if name is None: # the methods listed in 'REQUIRE_NAME_PARAMS' cannot have # 'name' == None @@ -88,7 +90,13 @@ class StorageVolumesModel(object): if create_param == 'file': name = os.path.basename(params['file'].filename) elif create_param == 'url': - name = os.path.basename(params['url']) + basename = os.path.basename(params['url']) + if basename in all_vol_names: + prefix, suffix = os.path.splitext(basename) + name = get_next_clone_name(all_vol_names, + prefix, suffix, '') + else: + name = basename else: name = 'upload-%s' % int(time.time()) params['name'] = name @@ -106,7 +114,7 @@ class StorageVolumesModel(object): if pool_info['state'] == 'inactive': raise InvalidParameter('KCHVOL0003E', {'pool': pool_name, 'volume': name}) - if name in self.get_list(pool_name): + if name in all_vol_names: raise InvalidParameter('KCHVOL0001E', {'name': name}) params['pool'] = pool_name -- 1.9.3
On 12/10/2014 01:17 AM, Crístian Viana wrote:
If the user downloads multiple remote files with the same name, only the first one will be completed; the following attempts will fail because a volume with the name already exists.
The following commands demonstrate the issue:
$ kimchi_rest /storagepools/default/storagevolumes | grep name "name":"60f46fdc-8ecc-4601-90d6-b3b44d35c337-0.img", "name":"bdd22e7c-7794-4bdf-8d2b-0089242c8b16-0.img", "name":"c16a7e02-6dcf-48c5-83de-1bb2d8a110a8-0.img",
$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"2", "target_uri":"/storagepools/default/storagevolumes/index.html" }
$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "reason":"KCHVOL0001E: Storage volume index.html already exists", "code":"400 Bad Request", "call_stack":"Traceback (most recent call last):\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, in respond\n response.body = self.handler()\n File \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 188, in __call__\n self.body = self.oldhandler(*args, **kwargs)\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, in __call__\n return self.callable(*self.args, **self.kwargs)\n File \"/home/vianac/LTC/kimchi/src/kimchi/control/base.py\", line 331, in index\n raise cherrypy.HTTPError(400, e.message)\nHTTPError: (400, u'KCHVOL0001E: Storage volume index.html already exists')\n"
Detect whether the file to be downloaded will have a name collision with the existing volumes and generate a different name in that case.
The following commands demonstrate the bug fix:
$ kimchi_rest /storagepools/default/storagevolumes | grep name "name":"60f46fdc-8ecc-4601-90d6-b3b44d35c337-0.img", "name":"bdd22e7c-7794-4bdf-8d2b-0089242c8b16-0.img", "name":"c16a7e02-6dcf-48c5-83de-1bb2d8a110a8-0.img",
$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"1", "target_uri":"/storagepools/default/storagevolumes/index.html" }
$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"2", "target_uri":"/storagepools/default/storagevolumes/index-1.html" }
$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"3", "target_uri":"/storagepools/default/storagevolumes/index-2.html"
Fix issue #543 ("Download a volume with same basename will result error").
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/model/storagevolumes.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 406b38b..0b8bc15 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -76,6 +76,8 @@ class StorageVolumesModel(object): except: raise InvalidParameter('KCHVOL0022E', {'url': url})
+ all_vol_names = self.get_list(pool_name) + if name is None: # the methods listed in 'REQUIRE_NAME_PARAMS' cannot have # 'name' == None @@ -88,7 +90,13 @@ class StorageVolumesModel(object): if create_param == 'file': name = os.path.basename(params['file'].filename) elif create_param == 'url':
- name = os.path.basename(params['url']) + basename = os.path.basename(params['url']) + if basename in all_vol_names: + prefix, suffix = os.path.splitext(basename) + name = get_next_clone_name(all_vol_names, + prefix, suffix, '') + else: + name = basename
This approach should work in all cases when creating a new storage volume (ie, file, url and capacity)
else: name = 'upload-%s' % int(time.time()) params['name'] = name @@ -106,7 +114,7 @@ class StorageVolumesModel(object): if pool_info['state'] == 'inactive': raise InvalidParameter('KCHVOL0003E', {'pool': pool_name, 'volume': name}) - if name in self.get_list(pool_name): + if name in all_vol_names: raise InvalidParameter('KCHVOL0001E', {'name': name})
params['pool'] = pool_name
On 10-12-2014 09:36, Aline Manera wrote:
This approach should work in all cases when creating a new storage volume (ie, file, url and capacity)
OK. But if the user explicitly sets a name, should we also try to change it like that? IMO, we should use this approach for all storage volume types only when the user doesn't set a name and let Kimchi pick one.
On 12/10/2014 01:59 PM, Crístian Viana wrote:
On 10-12-2014 09:36, Aline Manera wrote:
This approach should work in all cases when creating a new storage volume (ie, file, url and capacity)
OK. But if the user explicitly sets a name, should we also try to change it like that? IMO, we should use this approach for all storage volume types only when the user doesn't set a name and let Kimchi pick one.
Agree
On 2014年12月11日 00:53, Aline Manera wrote:
On 12/10/2014 01:59 PM, Crístian Viana wrote:
On 10-12-2014 09:36, Aline Manera wrote:
This approach should work in all cases when creating a new storage volume (ie, file, url and capacity)
OK. But if the user explicitly sets a name, should we also try to change it like that? IMO, we should use this approach for all storage volume types only when the user doesn't set a name and let Kimchi pick one.
Agree
+1
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
If the user downloads multiple remote files with the same name, only the first one will be completed; the following attempts will fail because a volume with the name already exists.
The following commands demonstrate the issue:
$ kimchi_rest /storagepools/default/storagevolumes | grep name "name":"60f46fdc-8ecc-4601-90d6-b3b44d35c337-0.img", "name":"bdd22e7c-7794-4bdf-8d2b-0089242c8b16-0.img", "name":"c16a7e02-6dcf-48c5-83de-1bb2d8a110a8-0.img",
$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"2", "target_uri":"/storagepools/default/storagevolumes/index.html" }
$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "reason":"KCHVOL0001E: Storage volume index.html already exists", "code":"400 Bad Request", "call_stack":"Traceback (most recent call last):\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, in respond\n response.body = self.handler()\n File \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 188, in __call__\n self.body = self.oldhandler(*args, **kwargs)\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, in __call__\n return self.callable(*self.args, **self.kwargs)\n File \"/home/vianac/LTC/kimchi/src/kimchi/control/base.py\", line 331, in index\n raise cherrypy.HTTPError(400, e.message)\nHTTPError: (400, u'KCHVOL0001E: Storage volume index.html already exists')\n"
Detect whether the file to be downloaded will have a name collision with the existing volumes and generate a different name in that case.
The following commands demonstrate the bug fix:
$ kimchi_rest /storagepools/default/storagevolumes | grep name "name":"60f46fdc-8ecc-4601-90d6-b3b44d35c337-0.img", "name":"bdd22e7c-7794-4bdf-8d2b-0089242c8b16-0.img", "name":"c16a7e02-6dcf-48c5-83de-1bb2d8a110a8-0.img",
$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"1", "target_uri":"/storagepools/default/storagevolumes/index.html" }
$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"2", "target_uri":"/storagepools/default/storagevolumes/index-1.html" }
$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}' { "status":"running", "message":"OK", "id":"3", "target_uri":"/storagepools/default/storagevolumes/index-2.html"
Fix issue #543 ("Download a volume with same basename will result error").
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/model/storagevolumes.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 406b38b..0b8bc15 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -76,6 +76,8 @@ class StorageVolumesModel(object): except: raise InvalidParameter('KCHVOL0022E', {'url': url})
+ all_vol_names = self.get_list(pool_name) + if name is None: # the methods listed in 'REQUIRE_NAME_PARAMS' cannot have # 'name' == None @@ -88,7 +90,13 @@ class StorageVolumesModel(object): if create_param == 'file': name = os.path.basename(params['file'].filename) elif create_param == 'url': - name = os.path.basename(params['url']) + basename = os.path.basename(params['url']) + if basename in all_vol_names: + prefix, suffix = os.path.splitext(basename) For filename like "imgzip.tar.gz", we will get prefix for "imgzip.tar" and suffix ".gz"
On 2014年12月10日 11:17, Crístian Viana wrote: then name will be "imgzip.tar-clone-1.gz", because some image disk image may be compressed for download. Besides, I suggest we put the split together with name generation as all the split seems all the same.
+ name = get_next_clone_name(all_vol_names, + prefix, suffix, '') + else: + name = basename else: name = 'upload-%s' % int(time.time()) params['name'] = name @@ -106,7 +114,7 @@ class StorageVolumesModel(object): if pool_info['state'] == 'inactive': raise InvalidParameter('KCHVOL0003E', {'pool': pool_name, 'volume': name}) - if name in self.get_list(pool_name): + if name in all_vol_names: raise InvalidParameter('KCHVOL0001E', {'name': name})
params['pool'] = pool_name
On 11-12-2014 06:14, Royce Lv wrote:
For filename like "imgzip.tar.gz", we will get prefix for "imgzip.tar" and suffix ".gz" then name will be "imgzip.tar-clone-1.gz", because some image disk image may be compressed for download.
* the new name would be "imgzip.tar-1.gz" :) The "-clone" tag isn't added in that case. I agree that this is an "ugly" name, even though it solves the issue we're working on: not being able to download files with the same name. But what automatic rule can we use to generate a nice name for every file we download? Let's try it with "http://www.domain.com", "http://www.domain.com/foo", "http://www.domain.com/foo.zip", "http://www.domain.com/foo.tar.gz", "http://www.domain.com/foo.important.zip". It's hard to guess what the real extension is. I was looking at the behaviour of browsers (e.g. Chrome, Firefox), and I noticed they add " (1)" to the file name when it already exists. We can do that as well, even though I don't think it's standard to how we've been naming things in Kimchi: "my-template-vm-1", "my-vm-clone-1". I think "my-vol.tar.gz (1)" would look out of place. How about "my-vol.tar.gz-1" and ignore file extension completely? Do you have any other suggestion?
On 12/11/2014 11:15 AM, Crístian Viana wrote:
On 11-12-2014 06:14, Royce Lv wrote:
For filename like "imgzip.tar.gz", we will get prefix for "imgzip.tar" and suffix ".gz" then name will be "imgzip.tar-clone-1.gz", because some image disk image may be compressed for download.
* the new name would be "imgzip.tar-1.gz" :) The "-clone" tag isn't added in that case.
I agree that this is an "ugly" name, even though it solves the issue we're working on: not being able to download files with the same name. But what automatic rule can we use to generate a nice name for every file we download? Let's try it with "http://www.domain.com", "http://www.domain.com/foo", "http://www.domain.com/foo.zip", "http://www.domain.com/foo.tar.gz", "http://www.domain.com/foo.important.zip". It's hard to guess what the real extension is.
I was looking at the behaviour of browsers (e.g. Chrome, Firefox), and I noticed they add " (1)" to the file name when it already exists. We can do that as well, even though I don't think it's standard to how we've been naming things in Kimchi: "my-template-vm-1", "my-vm-clone-1". I think "my-vol.tar.gz (1)" would look out of place. How about "my-vol.tar.gz-1" and ignore file extension completely?
Do you have any other suggestion?
I prefer "my-vol.tar.gz (1)"
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 2014年12月11日 23:14, Aline Manera wrote:
On 12/11/2014 11:15 AM, Crístian Viana wrote:
On 11-12-2014 06:14, Royce Lv wrote:
For filename like "imgzip.tar.gz", we will get prefix for "imgzip.tar" and suffix ".gz" then name will be "imgzip.tar-clone-1.gz", because some image disk image may be compressed for download.
* the new name would be "imgzip.tar-1.gz" :) The "-clone" tag isn't added in that case.
I agree that this is an "ugly" name, even though it solves the issue we're working on: not being able to download files with the same name. But what automatic rule can we use to generate a nice name for every file we download? Let's try it with "http://www.domain.com", "http://www.domain.com/foo", "http://www.domain.com/foo.zip", "http://www.domain.com/foo.tar.gz", "http://www.domain.com/foo.important.zip". It's hard to guess what the real extension is.
I was looking at the behaviour of browsers (e.g. Chrome, Firefox), and I noticed they add " (1)" to the file name when it already exists. We can do that as well, even though I don't think it's standard to how we've been naming things in Kimchi: "my-template-vm-1", "my-vm-clone-1". I think "my-vol.tar.gz (1)" would look out of place. How about "my-vol.tar.gz-1" and ignore file extension completely?
Do you have any other suggestion?
I think if browser behavior is this, we can go that way or I just think linke imgzip-1.tar.gz is easier to manipulate (just need to split from the first dot), and this retain this extension leave system can automatically identify its extension and choose appropriate app to deal with it. May be I'm asking too much:), either way is OK for me.
I prefer "my-vol.tar.gz (1)"
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
participants (3)
-
Aline Manera -
Crístian Viana -
Royce Lv