Michael Pasternak has posted comments on this change.
Change subject: restapi: Do not return VM ticket if not needed
......................................................................
Patch Set 1: Do not submit
(3 inline comments)
-2 to entire patch, you have took a private use-case and matched entire
application for it, its should be exactly on
opposite, - your use-case should be matched to the application ...
all you need to fix this bug is check action response in
BackendVmResource.ticket() and if it not-succeeded (no matter what the reason
is), override the ticket with NULL,
e.g:
action.getTicket().setValue(null)
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendActionableResource.java
Line 73: protected Response doAction(final VdcActionType task, final
VdcActionParametersBase params, final Action action,
AbstractBackendResource.PollingType pollingType) {
Line 74: // generate ticket for set vm ticket action if case user did
not supply it
Line 75: final boolean ticketExpected = (task ==
VdcActionType.SetVmTicket) && (action.getTicket().getValue() == null);
Line 76: if(ticketExpected) {
Line 77: action.getTicket().setValue(GenerateOTP());
why do you need that? SetVmTicketCommand can generate the ticket if not
provided by user ...,
also you've removed this code from ticket() impl and adding it here? in generic
doAction() ?
Line 78: }
Line 79:
Line 80: awaitGrace(action);
Line 81: try {
Line 75: final boolean ticketExpected = (task ==
VdcActionType.SetVmTicket) && (action.getTicket().getValue() == null);
Line 76: if(ticketExpected) {
Line 77: action.getTicket().setValue(GenerateOTP());
Line 78: }
Line 79:
this code does not belong to the Abstract.doAction(), this is private use-case.
Line 80: awaitGrace(action);
Line 81: try {
Line 82: VdcReturnValueBase actionResult = doAction(task, params);
Line 83: if (actionResult.getHasAsyncTasks()) {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java
Line 188: VdcActionParametersBase
params) throws BackendFailureException {
Line 189: setCorrelationId(params);
Line 190: VdcReturnValueBase result = backend.RunAction(task,
sessionize(params));
Line 191: if (!result.getCanDoAction()) {
Line 192: throw new
BackendNotAuthorizedException(localize(result.getCanDoActionMessages()));
who told that every BE can-do-action failure is caused by NotAuthorized?,
please revert this change.
Line 193: } else if (!result.getSucceeded()) {
Line 194: throw new
BackendFailureException(localize(result.getExecuteFailedMessages()));
Line 195: }
Line 196: assert (result != null);
--
To view, visit http://gerrit.ovirt.org/9855
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I187a95105f3dc7bb56ccf8dfa361f51a32b86fb6
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Libor Spevak <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
