I took a look at this earlier. Thanks for incorporating the feedback. The tests in your backport fail for me (see below), but otherwise, unless there are objections, I think we should merge this in once those tests are fixed.
Thanks Jon On Wed, Jul 12, 2017 at 6:00 PM, Otávio Gonçalves de Santana < osant...@tomitribe.com> wrote: > Thanks Romain, Jonathan. > > I did this change and also put more tests: > > https://github.com/apache/tomee/pull/89 > > > > On Wed, Jul 12, 2017 at 10:16 AM, Romain Manni-Bucau < > rmannibu...@gmail.com> > wrote: > > > SystemInstance.get().getComponent(ContainerSystem. > class).getContainer(id) > > > > Since AutoConfig should pass before it should be set and if not we should > > just move the override dynamicdeployer at the right place to have it set > > and still be read by its consumers. In any case, not splitting the > override > > logic is key for maintenance and ensure it is done in the right order > > (which is something to ensure we test - override priorities) > > > > > > Romain Manni-Bucau > > @rmannibucau <https://twitter.com/rmannibucau> | Blog > > <https://blog-rmannibucau.rhcloud.com> | Old Blog > > <http://rmannibucau.wordpress.com> | Github <https://github.com/ > > rmannibucau> | > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory > > <https://javaeefactory-rmannibucau.rhcloud.com> > > > > 2017-07-12 15:04 GMT+02:00 Otávio Gonçalves de Santana < > > osant...@tomitribe.com>: > > > > > String containerId = ejbDeployment.getContainerId(); > > > > > > It returns a container id, > > > How can I return the container from that? > > > > > > On Wed, Jul 12, 2017 at 9:40 AM, Romain Manni-Bucau < > > rmannibu...@gmail.com > > > > > > > wrote: > > > > > > > org.apache.openejb.jee.oejb3.EjbDeployment#getContainerId should > give > > > you > > > > the container > > > > > > > > > > > > Romain Manni-Bucau > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog > > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog > > > > <http://rmannibucau.wordpress.com> | Github <https://github.com/ > > > > rmannibucau> | > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory > > > > <https://javaeefactory-rmannibucau.rhcloud.com> > > > > > > > > 2017-07-12 14:29 GMT+02:00 Otávio Gonçalves de Santana < > > > > osant...@tomitribe.com>: > > > > > > > > > 1) Getter removed, thanks > > > > > 2) I took a look at that. As far as I can see, we don't have the > > > > container > > > > > (or its ID) available at the time when > ActivationConfigPropertyOverri > > > de. > > > > > Please let us know if we've missed something there. > > > > > > > > > > On Wed, Jul 12, 2017 at 9:17 AM, Romain Manni-Bucau < > > > > rmannibu...@gmail.com > > > > > > > > > > > wrote: > > > > > > > > > > > Hi Otavio, > > > > > > > > > > > > I have 2 notes on this: > > > > > > > > > > > > 1. the getProperties() is useless I think (IDE generated?) > > > > > > 2. it would probably better fit ActivationConfigPropertyOverride > > to > > > > > have a > > > > > > right ordering of overrides, no? > > > > > > > > > > > > > > > > > > Romain Manni-Bucau > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog > > > > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog > > > > > > <http://rmannibucau.wordpress.com> | Github <https://github.com/ > > > > > > rmannibucau> | > > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE > > Factory > > > > > > <https://javaeefactory-rmannibucau.rhcloud.com> > > > > > > > > > > > > 2017-07-12 13:42 GMT+02:00 Otávio Gonçalves de Santana < > > > > > > osant...@tomitribe.com>: > > > > > > > > > > > > > I have created this resource to both master and backported. > > > > > > > > > > > > > > > > > > > > > - https://github.com/apache/tomee/pull/90 > > > > > > > - https://github.com/apache/tomee/pull/89 > > > > > > > > > > > > > > > > > > > > > On Mon, Jul 10, 2017 at 7:33 AM, Jonathan Gallimore < > > > > > > > jonathan.gallim...@gmail.com> wrote: > > > > > > > > > > > > > > > We have a +1 and a +0 for the backport. I'm pulling the > latest > > > code > > > > > and > > > > > > > > running the tests. If it looks ok, I'll merge it, while > Otavio > > > > works > > > > > on > > > > > > > the > > > > > > > > container-based config in another patch. Please shout if you > > have > > > > any > > > > > > > > objections. > > > > > > > > > > > > > > > > Otavio, let us know if you need any help or guidance on the > > > > container > > > > > > > based > > > > > > > > settings! > > > > > > > > > > > > > > > > Jon > > > > > > > > > > > > > > > > On Thu, Jul 6, 2017 at 9:44 PM, Otávio Gonçalves de Santana < > > > > > > > > osant...@tomitribe.com> wrote: > > > > > > > > > > > > > > > > > The problem > > > > > > > > > > > > > > > > > > The configuration for MDB activation properties allow any > > > > key/value > > > > > > to > > > > > > > be > > > > > > > > > specified. At present on the 1.7.x branch, the server will > > fail > > > > to > > > > > > > deploy > > > > > > > > > the application if the activation property is not present > on > > > the > > > > > > > > activation > > > > > > > > > spec class. > > > > > > > > > > > > > > > > > > This becomes painful in a scenario where more than one JMS > > > > resource > > > > > > > > > adapter/MDB container is used, and you wish to configure > the > > > > > > activation > > > > > > > > > properties of multiple MDBs in one go using the > > > `mdb.activation.` > > > > > > > system > > > > > > > > > property.. Right now,if an activation property is used that > > one > > > > > > > provider > > > > > > > > > uses but other one does, the server will throw an > exception. > > > > > > > > > > > > > > > > > > For example, given these parameters, > > > > > > > > > > > > > > > > > > - > > > > > > > > > > > > > > > > > > mdb.activation.ignore=foo > > > > > > > > > - > > > > > > > > > > > > > > > > > > mdb.activation.ignore2=bar > > > > > > > > > > > > > > > > > > > > > > > > > > > if ‘ignore’ and ‘ignore2’ are not present on an MDB’s > > > activation > > > > > spec > > > > > > > > > class, the following exception will be thrown. > > > > > > > > > > > > > > > > > > Caused by: org.apache.openejb.OpenEJBException: Error > > > deploying > > > > > > > > > 'Listener'. Exception: class org.apache.openejb. > > > > OpenEJBException: > > > > > > > > Unable > > > > > > > > > to create activation spec: No setter found for the > activation > > > > spec > > > > > > > > > properties: [ignore, ignore2]: Unable to create activation > > > spec: > > > > No > > > > > > > > setter > > > > > > > > > found for the activation spec properties: [ignore, ignore2] > > > > > > > > > > > > > > > > > > at > > > > > > > > > org.apache.openejb.assembler.classic.Assembler.startEjbs( > > > > > > > > > Assembler.java:1430) > > > > > > > > > > > > > > > > > > at > > > > > > > > > org.apache.openejb.assembler.classic.Assembler. > > > > > > > > > createApplication(Assembler.java:796) > > > > > > > > > > > > > > > > > > ... 19 more > > > > > > > > > > > > > > > > > > Caused by: org.apache.openejb.OpenEJBException: Unable to > > > create > > > > > > > > > activation > > > > > > > > > spec: No setter found for the activation spec properties: > > > > [ignore, > > > > > > > > ignore2] > > > > > > > > > > > > > > > > > > at > > > > > > > > > org.apache.openejb.core.mdb.MdbContainer. > > createActivationSpec( > > > > > > > > > MdbContainer.java:292) > > > > > > > > > > > > > > > > > > at org.apache.openejb.core.mdb.MdbContainer.deploy( > > > > > > > > > MdbContainer.java:159) > > > > > > > > > > > > > > > > > > at > > > > > > > > > org.apache.openejb.assembler.classic.Assembler.startEjbs( > > > > > > > > > Assembler.java:1417) > > > > > > > > > > > > > > > > > > ... 20 more > > > > > > > > > > > > > > > > > > Caused by: java.lang.IllegalArgumentException: No setter > > found > > > > for > > > > > > the > > > > > > > > > activation spec properties: [ignore, ignore2] > > > > > > > > > > > > > > > > > > at > > > > > > > > > org.apache.openejb.core.mdb.MdbContainer. > > createActivationSpec( > > > > > > > > > MdbContainer.java:262) > > > > > > > > > > > > > > > > > > ... 22 more > > > > > > > > > > > > > > > > > > > > > > > > > > > The solution > > > > > > > > > > > > > > > > > > The best solution to solve the communication among server > is > > to > > > > > put a > > > > > > > new > > > > > > > > > configuration property in TomEE. When this setting is > > enabled, > > > > > > > overriding > > > > > > > > > the FailOnUnknownActivationSpec attribute at > > > > > > > > > org.apache.openejb.core.mdb.MdbContainer class., that will > > be > > > > > > disabled > > > > > > > > by > > > > > > > > > default to don't break the compatibility, when the setter > > does > > > > not > > > > > > > exist > > > > > > > > it > > > > > > > > > put a log and then keep working. > > > > > > > > > > > > > > > > > > Basically, my proposal does a backport to 1.7 branch: > > > > > > > > > https://github.com/apache/tomee/commit/ > > > > > > 6522f349d0c31d6ec82e66378e0e55 > > > > > > > > > eded08aec0 > > > > > > > > > > > > > > > > > > Path: > > > > > > > > > > > > > > > > > > https://patch-diff.githubusercontent.com/raw/ > > > > > > apache/tomee/pull/86.diff > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
