Re: Please review PR #201

@Bruno: note that this is not what we are doing, I'm just mentionning that TomEE does not need that and that there is no need to put any pressure either on TomEE or Geronimo in such situation since everything is good on both side in current state. Romain Manni-Bucau @rmannibucau

Re: Please review PR #201

It's just that I would expect to release 1.0.1 for a mater of principle. I think we shouldn't throw away an already approved valid contribution. Bruno Baptista https://twitter.com/brunobat_ On 26/11/18 13:53, Romain Manni-Bucau wrote: Le lun. 26 nov. 2018 à 14:48, Bruno Baptista a écrit :

Re: Please review PR #201

Le lun. 26 nov. 2018 à 14:48, Bruno Baptista a écrit : > Hi Romain, > > We are holding other work with this discussion. > > Can we agree that this is good enough for a 1st version and move on with > a follow up PR?... It's not going to be worse than starting SE tasks > inside the container, like

Re: Please review PR #201

Hi Romain, We are holding other work with this discussion. Can we agree that this is good enough for a 1st version and move on with a follow up PR?... It's not going to be worse than starting SE tasks inside the container, like we have now. Also, releasing Safegard 1.0.1 would be nice.

Re: Please review PR #201

Le ven. 23 nov. 2018 à 16:34, Bruno Baptista a écrit : > Hi Romain, > > About "The point is not the cdi bean but the executor. So high level you > deploy an > app not using safeguard but it being present and you ensure the container > has no executor resource instantiated (you will get one (the

Re: Please review PR #201

Hi Romain, About "The point is not the cdi bean but the executor. So high level you deploy an app not using safeguard but it being present and you ensure the container has no executor resource instantiated (you will get one (the facade))." Sorry Romain, I still don't understand how the code

Re: Please review PR #201

Le ven. 23 nov. 2018 à 15:49, Bruno Baptista a écrit : > Hi Romain, > > Thanks for your comment. > > The class doing the resource injection is lazy loaded, specifically > /FailsafeContainerExecutionManagerProvider/. I did verify it in > development but no test was produced... And to say the

Re: Please review PR #201

Hi Romain, Thanks for your comment. The class doing the resource injection is lazy loaded, specifically /FailsafeContainerExecutionManagerProvider/. I did verify it in development but no test was produced... And to say the truth I wouldn't know how to validate if a bean has already been

Re: Please review PR #201

> It's lazily loaded, so no worries on that regard. What is "it" here? :) Conretely the bean instantiation yes cause it is normal scoped and the resource too cause it is by default lazy in tomee (service-jar.xml) but it is worth a test that prevent regression on that behavior IMHO, I didn't

Re: Please review PR #201

I’m ok with that. > On 23 Nov 2018, at 14:15, Bruno Baptista wrote: > > So... Jon suggested to me that the configurable resource can be done in an > additional PR by one of the new guys. I think it's an excellent ideal. > > If no one disagrees, I'll create a Jira with the details for it. > >

Re: Please review PR #201

Maybe add those config options in a second PR? What do you think? Jon On Fri, Nov 23, 2018 at 2:01 PM Bruno Baptista wrote: > Hi Romain, > > In the end I decided to simply use the server default, for now. > > It will only be used if annotations are called in the code. It's lazily > loaded, so

Re: Please review PR #201

So... Jon suggested to me that the configurable resource can be done in an additional PR by one of the new guys. I think it's an excellent ideal. If no one disagrees, I'll create a Jira with the details for it. Bruno Baptista https://twitter.com/brunobat_ On 23/11/18 14:05, Roberto Cortez

Re: Please review PR #201

If we are happy, I would like to merge it. I could use some of the common project setup for other work. > On 23 Nov 2018, at 14:01, Bruno Baptista wrote: > > Hi Romain, > > In the end I decided to simply use the server default, for now. > > It will only be used if annotations are called in

Re: Please review PR #201

Hi Romain, In the end I decided to simply use the server default, for now. It will only be used if annotations are called in the code. It's lazily loaded, so no worries on that regard. Cheers. Bruno Baptista https://twitter.com/brunobat_ On 23/11/18 12:31, Romain Manni-Bucau wrote: Didnt

Re: Please review PR #201

Thanks! Additionally, I've requested a Safeguard 1.0.1 release. we shouldn't be using snapshots. Cheers Bruno Baptista https://twitter.com/brunobat_ On 22/11/18 19:30, Roberto Cortez wrote: Cool! Thank you. I’ll have a look. On 22 Nov 2018, at 19:08, Bruno Baptista wrote: Hi! I

Re: Please review PR #201

Cool! Thank you. I’ll have a look. > On 22 Nov 2018, at 19:08, Bruno Baptista wrote: > > Hi! > > I think the code is ready. Can some of you please review this pull request: > > https://github.com/apache/tomee/pull/201 > > Related to:TOMEE-2278

Please review PR #201

Hi! I think the code is ready. Can some of you please review this pull request: https://github.com/apache/tomee/pull/201 Related to:TOMEE-2278 - Use Managed Executor with Safeguard Fault Tolerance lib Cheers. -- Bruno Baptista