----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5625/#review9201 -----------------------------------------------------------
The following needs to be fixed: 1) CSExceptionErrorCode class. Never define the references to class as String values because it might be broken in the future by re-packaging. Please change all these errors: ExceptionErrorCodeMap.put("com.cloud.utils.exception.CloudRuntimeException", 4250); ExceptionErrorCodeMap.put("com.cloud.utils.exception.ExceptionUtil", 4255); ExceptionErrorCodeMap.put("com.cloud.utils.exception.ExecutionException", 4260); ExceptionErrorCodeMap.put("com.cloud.utils.exception.HypervisorVersionChangedException", 4265); This comment would be irrelevant thought once #2 is resolved. Just noting it so you would be aware of issues like that in the future. 2) I think you should refactor the way you get the exception error code in CloudRuntimeException. You use reflection, and this is very costly: public CloudException(String message) { super(message); setCSErrorCode(CSExceptionErrorCode.getCSErrCode(this.getClass().getName())); } I would do it as follows instead: * define CloudStackException interface (or name it the way you want). Define getErrorCode() method in there, make ALL cloudStack * define errorCode in each exception class That would be much cleaner and proper way to do it. Do the same for RuntimeCloudException 3) ApiDispatcher: if (idList != null) { + // Iterate through entire arraylist and copy over each proxy id. + for (int i = 0 ; i < idList.size(); i++) { + IdentityProxy id = idList.get(i); + ex.addProxyObject(id.getTableName(), id.getValue(), id.getidFieldName()); + s_logger.info(t.getMessage() + " db_id: " + id.getValue()); + } Currently you have to iterate through the list of identity proxies. Why don't you just set the Exception's with List<IdentityProxy>? It would be much cleaner, and you don't have to repeat the iteration every place in the code - Alena Prokharchyk On June 28, 2012, 12:20 a.m., Venkata Siva Vijayendra Bhamidipati wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5625/ > ----------------------------------------------------------- > > (Updated June 28, 2012, 12:20 a.m.) > > > Review request for cloudstack and Alena Prokharchyk. > > > Description > ------- > > Fix for bug CS-15372 (http://bugs.cloudstack.org/browse/CS-15372). > > > This addresses bug CS-15372. > > > Diffs > ----- > > api/src/com/cloud/exception/CloudException.java 9ebeaad > server/src/com/cloud/api/ApiDispatcher.java 0079830 > utils/src/com/cloud/utils/exception/RuntimeCloudException.java f2e9845 > > Diff: https://reviews.apache.org/r/5625/diff/ > > > Testing > ------- > > > Thanks, > > Venkata Siva Vijayendra Bhamidipati > >