mooli tayer has posted comments on this change.

Change subject: core: Introduce CDI for Commands dependencies
......................................................................


Patch Set 13:

(2 comments)

I also agree that this change is very important. 
just yesterday I encountered a problem with bad implementation of singleton 
using static
(some testing issue, will not go into it here ) this way is the current 
standard.

I have two comments:
A.)
I could not understand why we need BllCDIAdapter?
Isn't it better to put @Produces in the dependent class?

That way we have @Produces where we create something,
and @Inject where we need it.

If we need to update this class when ever we want to add a new 
dependency to be injected in bll it goes against adding a managed infra?

B.)
Regarding injector, 
Another option is to inject needed components in the injecting class 
(CommandsFactory here)
And inject them manually. thus avoiding mixing managed and non managed 
environments.

To summarize I think we need as lees infra around CDI as possible,
on the other hand I'm sure you looked into it  in depth so you probably
see problems I do not. please explain those.

http://gerrit.ovirt.org/#/c/5575/13/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 306: ();
So the default behavior is to assume injected beans are != 
null?

If an injected bean is missing what will happen? the container will warn and 
abort?


Line 833: ActionVersionMap
remove variable?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f604ff91847b698efe84a09f724ba0492a672c1
Gerrit-PatchSet: 13
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Asaf Shakarchi <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: mooli tayer <[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