Mike Kolesnik has posted comments on this change.
Change subject: engine: introducing RefreshHostCapabilitiesCommand
......................................................................
Patch Set 9: (3 inline comments)
Some very small comments.
Additionally, please add type 'host information' for the action type, since
this would be much more informative to the user when he sees 'Cannot refresh
host information' rather than 'Cannot refresh host' which could mean a whole
bunch of stuff.
Alternatively you could say 'host capabilities' but since it's an internal
term, not sure if the users would understand it or not.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RefreshHostCapabilitiesCommand.java
Line 36: return validate(hostExists()) && validate(hostStatusValid());
Line 37: }
Line 38:
Line 39: private ValidationResult hostExists() {
Line 40: if (getVds() == null) {
You could use ternary if for this:
return getVds() == null ? new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NOT_EXIST) :
ValidationResult.VALID;
Line 41: return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NOT_EXIST);
Line 42: }
Line 43:
Line 44: return ValidationResult.VALID;
Line 46:
Line 47: private ValidationResult hostStatusValid() {
Line 48: VDSStatus hostStatus = getVds().getStatus();
Line 49: if (hostStatus != VDSStatus.Maintenance && hostStatus !=
VDSStatus.Up) {
Line 50:
addCanDoActionMessage(VdcBllMessages.VAR__HOST_STATUS__UP_OR_MAINTENANCE);
You don't have to use addCanDoActionMessage for this, since there's a
ValidationResult constructor that receives a list or replacements (ellipsis so
you don't really have to wrap it in a list or anything).
That's the way it is used in most places
Line 51: return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL);
Line 52: }
Line 53:
Line 54: return ValidationResult.VALID;
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 285: VAR__ACTION__DISABLE=$action disable
Line 286: VAR__ACTION__REFRESH=$action refresh
Line 287: VAR__HOST_STATUS__UP=$hostStatus Up
Line 288: VAR__HOST_STATUS__UP_MAINTENANCE_OR_NON_OPERATIONAL=$hostStatus Up,
Maintenance or Non operational
Line 289: VAR__HOST_STATUS__UP_OR_MAINTENANCE=$hostStatus Up or Maintenance
The statuses should be in single quotes, i.e. 'Up' or 'Maintenance'
Line 290:
Line 291: ACTION_TYPE_FAILED_DISK_ALREADY_ATTACHED=Cannot ${action} ${type}.
The disk is already attached to VM.
Line 292: ACTION_TYPE_FAILED_DISK_ALREADY_DETACHED=Cannot ${action} ${type}.
The disk is already detached from VM.
Line 293: ACTION_TYPE_FAILED_NOT_SHAREABLE_DISK_ALREADY_ATTACHED=Cannot
${action} ${type}. The disk is not shareable and is already attached to a VM.
--
To view, visit http://gerrit.ovirt.org/15640
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba25577a7b66f3acedc3b32b258560861d19c621
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
