[PATCH 0/4] Fix: Report Overwritten when Provided Name Existing
When user provides an existing debug report name, then the server will generate a new debug report and overwrite the old one. Fixed it in this patch to detect the existing name and ask the user to choose another name. Depends on following patch: * [PATCH] Bugfix: Multiple progress indicator during debug report generating Hongliang Wang (4): Add ResourceAlreadyExists Exception (HTTP 409) Add Name Existence Check for Debug Report when Create/Rename Add License Statement in kimchi.report_add_main.js Implement UI Part to Properly Handle Report Name Existence src/kimchi/control/base.py | 3 +++ src/kimchi/exception.py | 4 ++++ src/kimchi/i18n.py | 1 + src/kimchi/model/debugreports.py | 12 +++++++++- ui/js/src/kimchi.report_add_main.js | 45 ++++++++++++++++++++++++++++--------- 5 files changed, 54 insertions(+), 11 deletions(-) -- 1.8.1.4
When user tries to create a resource with an existing name, we should return HTTP 409 status code to tell the user and let him/her choose another one. Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 3 +++ src/kimchi/exception.py | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index f8a5210..aaa479d 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,6 +28,7 @@ from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import KimchiException from kimchi.exception import MissingParameter, NotFoundError, OperationFailed +from kimchi.exception import ResourceAlreadyExists class Resource(object): @@ -84,6 +85,8 @@ class Resource(object): raise cherrypy.HTTPError(400, e.message) except InvalidOperation, e: raise cherrypy.HTTPError(400, e.message) + except ResourceAlreadyExists, e: + raise cherrypy.HTTPError(409, e.message) except OperationFailed, e: raise cherrypy.HTTPError(500, e.message) except NotFoundError, e: diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index fcf60cc..38daeab 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -91,3 +91,7 @@ class IsoFormatError(KimchiException): class TimeoutExpired(KimchiException): pass + + +class ResourceAlreadyExists(KimchiException): + pass -- 1.8.1.4
On 06/12/2014 04:07 AM, Hongliang Wang wrote:
When user tries to create a resource with an existing name, we should return HTTP 409 status code to tell the user and let him/her choose another one.
Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 3 +++ src/kimchi/exception.py | 4 ++++ 2 files changed, 7 insertions(+)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index f8a5210..aaa479d 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,6 +28,7 @@ from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import KimchiException from kimchi.exception import MissingParameter, NotFoundError, OperationFailed +from kimchi.exception import ResourceAlreadyExists
class Resource(object): @@ -84,6 +85,8 @@ class Resource(object): raise cherrypy.HTTPError(400, e.message) except InvalidOperation, e: raise cherrypy.HTTPError(400, e.message) + except ResourceAlreadyExists, e: + raise cherrypy.HTTPError(409, e.message) except OperationFailed, e: raise cherrypy.HTTPError(500, e.message) except NotFoundError, e: diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index fcf60cc..38daeab 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -91,3 +91,7 @@ class IsoFormatError(KimchiException):
class TimeoutExpired(KimchiException): pass + + +class ResourceAlreadyExists(KimchiException): + pass
You don't need to create a new exception type In those situation, we use InvalidParameter()
On 06/13/2014 02:03 AM, Aline Manera wrote:
On 06/12/2014 04:07 AM, Hongliang Wang wrote:
When user tries to create a resource with an existing name, we should return HTTP 409 status code to tell the user and let him/her choose another one.
Signed-off-by: Hongliang Wang<hlwang@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 3 +++ src/kimchi/exception.py | 4 ++++ 2 files changed, 7 insertions(+)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index f8a5210..aaa479d 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,6 +28,7 @@ from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import KimchiException from kimchi.exception import MissingParameter, NotFoundError, OperationFailed +from kimchi.exception import ResourceAlreadyExists
class Resource(object): @@ -84,6 +85,8 @@ class Resource(object): raise cherrypy.HTTPError(400, e.message) except InvalidOperation, e: raise cherrypy.HTTPError(400, e.message) + except ResourceAlreadyExists, e: + raise cherrypy.HTTPError(409, e.message) except OperationFailed, e: raise cherrypy.HTTPError(500, e.message) except NotFoundError, e: diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index fcf60cc..38daeab 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -91,3 +91,7 @@ class IsoFormatError(KimchiException):
class TimeoutExpired(KimchiException): pass + + +class ResourceAlreadyExists(KimchiException): + pass
You don't need to create a new exception type In those situation, we use InvalidParameter() I noticed that and was to use InvalidParameter() though I found it only returns 400 which is not that specific for existing resource error. When we use InvalidParameter with HTTP status code 400, the user can only knows some parameter is wrong which is relatively general. If we use 409, the use will definitely know the given name conflicts with an existing one. So I suggest 409 here.
Prevent user overwrite an existing debug report with same name. Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/debugreports.py | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 452ede2..90ecc17 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -207,6 +207,7 @@ messages = { "KCHDR0005E": _("Unable to generate debug report %(name)s. Details: %(err)s"), "KCHDR0006E": _("You should give a name for the debug file report."), "KCHDR0007E": _("Name should be a string. Only letters, digits and hyphen ('-') are allowed."), + "KCHDR0008E": _("The debug report with specified name \"%(name)s\" already exists. Please use another one."), "KCHSR0001E": _("Storage server %(server)s was not used by Kimchi"), diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py index cd31b31..3aa0fdc 100644 --- a/src/kimchi/model/debugreports.py +++ b/src/kimchi/model/debugreports.py @@ -26,7 +26,7 @@ import subprocess import time from kimchi import config -from kimchi.exception import KimchiException, NotFoundError, OperationFailed +from kimchi.exception import KimchiException, NotFoundError, OperationFailed, ResourceAlreadyExists from kimchi.model.tasks import TaskModel from kimchi.utils import add_task, kimchi_log from kimchi.utils import run_command @@ -42,6 +42,9 @@ class DebugReportsModel(object): # Generate a name with time and millisec precision, if necessary if ident is None or ident == "": ident = 'report-' + str(int(time.time() * 1000)) + else: + self._check_existence(ident) + taskid = self._gen_debugreport_file(ident) return self.task.lookup(taskid) @@ -54,6 +57,13 @@ class DebugReportsModel(object): return name_lists + def _check_existence(self, name): + path = config.get_debugreports_path() + file_pattern = os.path.join(path, name + '.*') + exists = [f for f in glob.glob(file_pattern) if os.path.isfile(f)] + if exists: + raise ResourceAlreadyExists("KCHDR0008E", {"name": name}) + def _gen_debugreport_file(self, name): gen_cmd = self.get_system_report_tool() -- 1.8.1.4
On 06/12/2014 04:07 AM, Hongliang Wang wrote:
Prevent user overwrite an existing debug report with same name.
Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/debugreports.py | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 452ede2..90ecc17 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -207,6 +207,7 @@ messages = { "KCHDR0005E": _("Unable to generate debug report %(name)s. Details: %(err)s"), "KCHDR0006E": _("You should give a name for the debug file report."), "KCHDR0007E": _("Name should be a string. Only letters, digits and hyphen ('-') are allowed."), + "KCHDR0008E": _("The debug report with specified name \"%(name)s\" already exists. Please use another one."),
"KCHSR0001E": _("Storage server %(server)s was not used by Kimchi"),
diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py index cd31b31..3aa0fdc 100644 --- a/src/kimchi/model/debugreports.py +++ b/src/kimchi/model/debugreports.py @@ -26,7 +26,7 @@ import subprocess import time
from kimchi import config -from kimchi.exception import KimchiException, NotFoundError, OperationFailed +from kimchi.exception import KimchiException, NotFoundError, OperationFailed, ResourceAlreadyExists from kimchi.model.tasks import TaskModel from kimchi.utils import add_task, kimchi_log from kimchi.utils import run_command @@ -42,6 +42,9 @@ class DebugReportsModel(object): # Generate a name with time and millisec precision, if necessary if ident is None or ident == "": ident = 'report-' + str(int(time.time() * 1000)) + else: + self._check_existence(ident) + taskid = self._gen_debugreport_file(ident) return self.task.lookup(taskid)
@@ -54,6 +57,13 @@ class DebugReportsModel(object):
return name_lists
+ def _check_existence(self, name): + path = config.get_debugreports_path() + file_pattern = os.path.join(path, name + '.*') + exists = [f for f in glob.glob(file_pattern) if os.path.isfile(f)] + if exists: + raise ResourceAlreadyExists("KCHDR0008E", {"name": name}) +
You can do: if name in self.get_list(): raise InvalidParameter()
def _gen_debugreport_file(self, name): gen_cmd = self.get_system_report_tool()
On 06/13/2014 02:03 AM, Aline Manera wrote:
On 06/12/2014 04:07 AM, Hongliang Wang wrote:
Prevent user overwrite an existing debug report with same name.
Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/debugreports.py | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 452ede2..90ecc17 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -207,6 +207,7 @@ messages = { "KCHDR0005E": _("Unable to generate debug report %(name)s. Details: %(err)s"), "KCHDR0006E": _("You should give a name for the debug file report."), "KCHDR0007E": _("Name should be a string. Only letters, digits and hyphen ('-') are allowed."), + "KCHDR0008E": _("The debug report with specified name \"%(name)s\" already exists. Please use another one."),
"KCHSR0001E": _("Storage server %(server)s was not used by Kimchi"),
diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py index cd31b31..3aa0fdc 100644 --- a/src/kimchi/model/debugreports.py +++ b/src/kimchi/model/debugreports.py @@ -26,7 +26,7 @@ import subprocess import time
from kimchi import config -from kimchi.exception import KimchiException, NotFoundError, OperationFailed +from kimchi.exception import KimchiException, NotFoundError, OperationFailed, ResourceAlreadyExists from kimchi.model.tasks import TaskModel from kimchi.utils import add_task, kimchi_log from kimchi.utils import run_command @@ -42,6 +42,9 @@ class DebugReportsModel(object): # Generate a name with time and millisec precision, if necessary if ident is None or ident == "": ident = 'report-' + str(int(time.time() * 1000)) + else: + self._check_existence(ident) + taskid = self._gen_debugreport_file(ident) return self.task.lookup(taskid)
@@ -54,6 +57,13 @@ class DebugReportsModel(object):
return name_lists
+ def _check_existence(self, name): + path = config.get_debugreports_path() + file_pattern = os.path.join(path, name + '.*') + exists = [f for f in glob.glob(file_pattern) if os.path.isfile(f)] + if exists: + raise ResourceAlreadyExists("KCHDR0008E", {"name": name}) +
You can do:
if name in self.get_list(): raise InvalidParameter() ACK. It's useful. Thanks!
def _gen_debugreport_file(self, name): gen_cmd = self.get_system_report_tool()
Added license statement. Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- ui/js/src/kimchi.report_add_main.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/ui/js/src/kimchi.report_add_main.js b/ui/js/src/kimchi.report_add_main.js index ad2bb39..28c3e66 100644 --- a/ui/js/src/kimchi.report_add_main.js +++ b/ui/js/src/kimchi.report_add_main.js @@ -1,3 +1,20 @@ +/* + * Project Kimchi + * + * Copyright IBM, Corp. 2014 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ kimchi.report_add_main = function() { var reportGridID = 'available-reports-grid'; var addReportForm = $('#form-report-add'); -- 1.8.1.4
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 06/12/2014 04:07 AM, Hongliang Wang wrote:
Added license statement.
Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- ui/js/src/kimchi.report_add_main.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/ui/js/src/kimchi.report_add_main.js b/ui/js/src/kimchi.report_add_main.js index ad2bb39..28c3e66 100644 --- a/ui/js/src/kimchi.report_add_main.js +++ b/ui/js/src/kimchi.report_add_main.js @@ -1,3 +1,20 @@ +/* + * Project Kimchi + * + * Copyright IBM, Corp. 2014 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ kimchi.report_add_main = function() { var reportGridID = 'available-reports-grid'; var addReportForm = $('#form-report-add');
participants (2)
-
Aline Manera -
Hongliang Wang