Sergey Gotliv has posted comments on this change.

Change subject: engine: Remove calls to getImageDomainsList API
......................................................................


Patch Set 3:

(2 comments)

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
Line 780:     
ACTION_TYPE_FAILED_CANNOT_REMOVE_ACTIVE_DEVICE(ErrorType.CONFLICT),
Line 781:     
ACTION_TYPE_FAILED_IMPORT_CLONE_NOT_COLLAPSED(ErrorType.BAD_PARAMETERS),
Line 782: 
Line 783:     /* VDSM Error that propagates up to the client. See VdcBLLErrors 
*/
Line 784:     ERROR_GET_STORAGE_DOMAIN_LIST(ErrorType.INTERNAL_ERROR), // 
VdcBllErrors.GetStorageDomainListError
I will clean this error code and all related messages in another patch, because 
first I have to remove references to that error from GetImageListVDSCommand(the 
patch is already public) and GetVolumeListVDSCommand(next to come).
Line 785: 
Line 786:     ERROR_GET_IMAGE_LIST(ErrorType.INTERNAL_ERROR),
Line 787: 
Line 788:     USER_CANNOT_FORCE_RECONNECT_TO_VM(ErrorType.CONFLICT),


Line 782: 
Line 783:     /* VDSM Error that propagates up to the client. See VdcBLLErrors 
*/
Line 784:     ERROR_GET_STORAGE_DOMAIN_LIST(ErrorType.INTERNAL_ERROR), // 
VdcBllErrors.GetStorageDomainListError
Line 785: 
Line 786:     ERROR_GET_IMAGE_LIST(ErrorType.INTERNAL_ERROR),
My opinion is that validation is something that nice to have. But its important 
to remember that's the real job is move or copy not validate. If validation's 
result is inconclusive (something happened on VDSM side so I can't check if 
that image exists or not, this is a corner case) I prefer to continue to the 
real thing. In worst case scenario the real thing will fail either than I can 
say to user "we failed".
So, originally I was ready to swallow the error, but...

Before my change validation could return ERROR_GET_STORAGE_DOMAIN_LIST which is 
totally meaningless fro user perspective, now it will return this one instead.

I believe that our primary concern is behavior not the error message one or 
another.
Line 787: 
Line 788:     USER_CANNOT_FORCE_RECONNECT_TO_VM(ErrorType.CONFLICT),
Line 789:     // Engine maintenance mode
Line 790:     ENGINE_IS_RUNNING_IN_MAINTENANCE_MODE(ErrorType.CONFLICT),


-- 
To view, visit http://gerrit.ovirt.org/15951
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I33069871db34e8b129c4d80098523654cd2a0629
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: liron aravot <[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

Reply via email to