Moti Asayag has posted comments on this change.

Change subject: core: introduce context pattern
......................................................................


Patch Set 37:

(3 comments)

taking of -1, as the major concern about not passing context to created command 
and queries was addressed.

http://gerrit.ovirt.org/#/c/28829/37/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java:

Line 597:         getVmStaticDAO().incrementDbGeneration(getVmTemplateId());
Line 598:         for (VdcActionParametersBase p : 
getParameters().getImagesParameters()) {
Line 599:             
Backend.getInstance().endAction(VdcActionType.CreateImageTemplate,
Line 600:                     p,
Line 601:                     
getContext().duplicate().resetCompensationContext().resetExecutionContext().resetLock());
isn't there dupContext() method in CommandBase ?
Line 602:         }
Line 603:         if (reloadVmTemplateFromDB() != null) {
Line 604:             endDefaultOperations();
Line 605:         }


Line 675:         for (VdcActionParametersBase p : 
getParameters().getImagesParameters()) {
Line 676:             p.setTaskGroupSuccess(false);
Line 677:             
Backend.getInstance().endAction(VdcActionType.CreateImageTemplate,
Line 678:                     p,
Line 679:                     
getContext().duplicate().resetCompensationContext().resetExecutionContext().resetLock());
here as well
Line 680:         }
Line 681: 
Line 682:         // if template exist in db remove it
Line 683:         if (getVmTemplate() != null) {


http://gerrit.ovirt.org/#/c/28829/37/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java:

Line 170:     protected CommandBase(T parameters, CommandContext cmdContext) {
Line 171:         if (cmdContext == null) {
Line 172:             cmdContext =
Line 173:                     new CommandContext(new 
EngineContext().setSessionId(parameters.getSessionId()))
Line 174:                             .setExecutionContext(new 
ExecutionContext());
this is bit hard to follow. I'd extract the EngineContext into local variable:

   EngineContext engineContext = new 
EngineContext().withSessionId(parameters.getSessionId());
   cmdContext = new CommandContext()
                     .with(engineContext)
                     .with(new ExecutionContext());
Line 175:         }
Line 176:         this.context = cmdContext;
Line 177:         _parameters = parameters;
Line 178:         DbUser user =


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I310f5f77fff78b3232ee77fe63791425fd521516
Gerrit-PatchSet: 37
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[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

Reply via email to