Maor Lipchuk has posted comments on this change.

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


Patch Set 13: Code-Review+1

(3 comments)

http://gerrit.ovirt.org/#/c/5575/13/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandsFactory.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandsFactory.java:

Line 82:             if (!constructor.isAccessible()) {
Line 83:                 isAcessible = constructor.isAccessible();
Line 84:                 constructor.setAccessible(true);
Line 85:             }
Line 86:             CommandBase<?> cmd = (CommandBase<?>) 
constructor.newInstance(new Object[]{commandId});
please use a formatter on the code line
Line 87:             injector.inject(cmd);
Line 88:             return cmd;
Line 89:         } catch (Exception e) {
Line 90:             log.error(


http://gerrit.ovirt.org/#/c/5575/13/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/common/utils/Injector.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/common/utils/Injector.java:

Line 35:      *           members
Line 36:      */
Line 37:     public <T extends  Object> void inject(T instance) {
Line 38:         AnnotatedType type = 
manager.createAnnotatedType(instance.getClass());
Line 39:         manager.createInjectionTarget(type).inject(instance, 
manager.createCreationalContext(null));
Maybe using here a try catch block, will be good to track future problems, so 
we can know more details such as the type which was used to createInjection 
from and what was the problem.

Right now the commandsFactory is the only one that using it, and in case of a 
failure we will get "CommandsFactory : Failed to get type information using 
reflection for Class...." which could be confusing...
Line 40:     }
Line 41: 
Line 42:     /**
Line 43:      * This method will fetch a managed CDI bean from the CDI 
container.


Line 50:      * @return the instance of type <code><T></T></code> which is 
manged by the CDI container
Line 51:      */
Line 52:     public <T extends Object> T get(Class<T> clazz) {
Line 53:         Bean bean = manager.getBeans(clazz).iterator().next();
Line 54:         return (T) manager.getReference(bean, clazz, 
manager.createCreationalContext(bean));
same here
Line 55:     }


-- 
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