Allon Mureinik has posted comments on this change. Change subject: core: Throwing exception instead of handle fault ......................................................................
Patch Set 1: Code-Review-1 (5 comments) http://gerrit.ovirt.org/#/c/29259/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java: Line 43 Line 44 Line 45 Line 46 Line 47 success used to be modified here. but after your change... (see to the right) Line 30: } Line 31: Line 32: @Override Line 33: protected void executeCommand() { Line 34: boolean success = true; success is defined here... Line 35: Line 36: // Attempt to connect only if a host is given. Line 37: // If not, just save the connection to the database Line 38: if (!Guid.isNullOrEmpty(getParameters().getVdsId())) { Line 40: boolean isValidConnection = result.getFirst(); Line 41: Line 42: // Process failure Line 43: if (!isValidConnection) { Line 44: throw new VdcBLLException(VdcBllErrors.forValue(result.getSecond())); ... and no longer changed here. Line 45: } Line 46: } Line 47: Line 48: if (success) { Line 44: throw new VdcBLLException(VdcBllErrors.forValue(result.getSecond())); Line 45: } Line 46: } Line 47: Line 48: if (success) { so checking this is useless - it's known to be true. Line 49: StorageServerConnections connection = getConnection(); Line 50: connection.setid(Guid.newGuid().toString()); Line 51: saveConnection(connection); Line 52: getReturnValue().setActionReturnValue(connection.getid()); Line 50: connection.setid(Guid.newGuid().toString()); Line 51: saveConnection(connection); Line 52: getReturnValue().setActionReturnValue(connection.getid()); Line 53: } Line 54: and this should just be setSucceeded(true); Line 55: setSucceeded(success); Line 56: } Line 57: Line 58: protected StorageServerConnections getConnectionFromDbById(String connectionId) { -- To view, visit http://gerrit.ovirt.org/29259 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e6b8282d9e2cdd6e17f21a2ef034041d68d09fc Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Idan Shaby <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Idan Shaby <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
