Yep - I'd definitely do that in another PR. Jon
On Fri, Jul 7, 2017 at 12:30 PM, Otávio Gonçalves de Santana < osant...@tomitribe.com> wrote: > Sure, I can fix it on the master. > About the activation properties on a per-container basis, I agree, however > as a different diff path. > > On Fri, Jul 7, 2017 at 7:44 AM, Jonathan Gallimore < > jonathan.gallim...@gmail.com> wrote: > > > Thanks for the feedback Romain! @Otavio - what do you think? You want to > > have a go at that? > > > > Jon > > > > On Fri, Jul 7, 2017 at 11:41 AM, Romain Manni-Bucau < > rmannibu...@gmail.com > > > > > wrote: > > > > > sounds good this way for container activation props > > > > > > > > > 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-07 12:40 GMT+02:00 Jonathan Gallimore < > > > jonathan.gallim...@gmail.com> > > > : > > > > > > > I'm thinking something along the lines of: > > > > > > > > <Container id="MDB1" ctype="MESSAGE"> > > > > ResourceAdapter MyResourceAdapter > > > > ActivationSpecClass My.ActivationSpecImpl > > > > > > > > activation.property1 = value1 > > > > activation.property2 = value2 > > > > </Container> > > > > > > > > <Container id="MDB2" ctype="MESSAGE"> > > > > ResourceAdapter MyOTHERResourceAdapter > > > > ActivationSpecClass My.Other.ActivationSpecImpl > > > > activation.property1 = othervalue1 > > > > activation.property2 = othervalue2 > > > > </Container> > > > > > > > > So all the MDBs in container MDB1 would get the following on their > > > > activation spec > > > > property1 = value1 > > > > property2 = value2 > > > > > > > > And all the MDBs in container MDB2 would get the following on their > > > > activation spec > > > > property1 = othervalue1 > > > > property2 = othervalue2 > > > > > > > > > > > > And then have the potential to override them with system.properties > > like > > > > so: > > > > MDB1.activation.property1 = othervalue1 > > > > MDB1.activation.property2 = othervalue2 > > > > MDB2.activation.property1 = othervalue1 > > > > MDB2.activation.property2 = othervalue2 > > > > > > > > Maybe something is needed to call 'MDB1' out as a container as > opposed > > > to a > > > > bean name. In terms of precedence, I'd expect properties on the > > container > > > > to override any `mdb.activation` properties, and any properties > > specific > > > to > > > > a bean to override any container properties - so its Global > > > > (mdb.activation) -> Container ([containerid].activation) -> Bean > > > > ([BeanName].activation). > > > > > > > > I imagine it would essentially boil down to a new key at the point > you > > > > suggest, and expand on the test cases. > > > > > > > > I'm in favour of the backport irrespective of whether we choose to > > > explore > > > > my suggestion (or any other suggestion) though. > > > > > > > > Cheers > > > > > > > > Jon > > > > > > > > On Thu, Jul 6, 2017 at 10:09 PM, Romain Manni-Bucau < > > > rmannibu...@gmail.com > > > > > > > > > wrote: > > > > > > > > > 2017-07-06 23:01 GMT+02:00 Jonathan Gallimore < > > > jgallim...@tomitribe.com > > > > >: > > > > > > > > > > > I'm +1 for back porting that patch, thanks Otavio. > > > > > > > > > > > > One thing I'd be interested in as an extra, is seeing if we can > set > > > > > > activation properties on a per-container basis too. > > > > > > > > > > > > > > > > Mean like adding one new key in > > > > > https://github.com/apache/tomee/blob/master/container/ > > > > > openejb-core/src/main/java/org/apache/openejb/config/ > > > > > ActivationConfigPropertyOverride.java#L94 > > > > > using .container.<id>? > > > > > > > > > > +1 if ~so (just wanted to avoid a misunderstanding and get a > > completely > > > > new > > > > > feature/code) > > > > > > > > > > > > > > > > > > > > Side note on the fail flag: this was a flag added to pass tck and > was > > > in > > > > > "ignore" mode of the MDB (which is fine for this need) but not > > intended > > > > to > > > > > be used or reliable for real applications where it would be saner > to > > > fix > > > > > the broken configuration instead of bet on it working ignoring the > > work > > > > > some people did configuring it. I'm not against backporting it but > > > think > > > > it > > > > > is important to remind that I think we don't want to promote that > > flag > > > > > (shouldn't hit the doc for instance since it is an internal > > workaround > > > > > IMHO). > > > > > > > > > > > > > > > > > > > > > > What do you think? > > > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Jonathan Gallimore > > > > > > http://twitter.com/jongallimore > > > > > > http://www.tomitribe.com > > > > > > > > > > > > > > > > > > > > >
