Yair Zaslavsky has posted comments on this change. Change subject: WIP: working CDI solution for backend ......................................................................
Patch Set 6: (3 inline comments) Why do we need an empty beans.xml file? .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandsFactory.java Line 47: try { Line 48: Constructor<CommandBase<? extends VdcActionParametersBase>> constructor = Line 49: findCommandConstructor(getCommandClass(action.name(), CommandSuffix), parameters.getClass()); Line 50: Line 51: CommandBase<P> cmd = (CommandBase<P>) constructor.newInstance(new Object[] { parameters }); Please add a comment explaining why you still have to use cmd.inject and when it will be eliminated. Line 52: injector.inject(cmd); Line 53: return cmd; Line 54: } Line 55: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Injector.java Line 10: private BeanManager manager; Line 11: Line 12: public Injector() { Line 13: try { Line 14: this.manager = (BeanManager) new InitialContext().lookup("java:comp/BeanManager"); Can we use here the EJBUtils strategy? we use it to hide the lookups where possible. Just an idea... Line 15: } catch (NamingException e) { Line 16: // if the bean manager couldn't be fetched then we couldn't inject dependencies to non-bean objects Line 17: throw new RuntimeException("Couldn't locate a BeanManager instance due to " + e.getMessage() Line 18: + " Actions will fail to perform due to runtime missing dependencies"); Line 12: public Injector() { Line 13: try { Line 14: this.manager = (BeanManager) new InitialContext().lookup("java:comp/BeanManager"); Line 15: } catch (NamingException e) { Line 16: // if the bean manager couldn't be fetched then we couldn't inject dependencies to non-bean objects Please add log.debug for the stack trace. Line 17: throw new RuntimeException("Couldn't locate a BeanManager instance due to " + e.getMessage() Line 18: + " Actions will fail to perform due to runtime missing dependencies"); Line 19: } Line 20: } -- 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: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Asaf Shakarchi <a...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches