I would prefer to have a single module is possible. What I understood from Romain suggestion is to have a specific config that allows us to figure out what is the environment and then inject / lookup the right beans, is that correct?
> On 8 Oct 2018, at 14:19, Romain Manni-Bucau <[email protected]> wrote: > > Well the config point is tomee will still fork this bean cause it lust also > respect tomee config so id make all services either implicitly looked up or > injected in the producer and user has its own config properly instead of > creating a module misbehaving OOTB in 90% of the case and needinh another > piece of vonfig outside the app. > > Gain sounds quite low vs the cost in that perspective. > > Le lun. 8 oct. 2018 16:02, Bruno Baptista <[email protected] > <mailto:[email protected]>> a écrit : > Thanks, will play with the module and add tests. > Cheers > Bruno Baptista > http://twitter.com/brunobat_ <http://twitter.com/brunobat_> > > > On 08/10/2018 12:41, John D. Ament wrote: >> Agreed, do you want to add that module? And if so you can add a priority >> annotation to enable it by default. Would then also be good to add a test >> in an app server then. >> >> John >> >> >> On Mon, Oct 8, 2018, 06:46 Bruno Baptista <[email protected] >> <mailto:[email protected]>> wrote: >> Hi, >> >> I've updated the PR: https://github.com/apache/geronimo-safeguard/pull/2 >> <https://github.com/apache/geronimo-safeguard/pull/2> >> Now... we should probably change the project structure and have an impl >> artifact for Java SE and another one for the enterprise edition, using the >> different ExecutionManagerProvider implementations. >> What do you guys think? >> >> Cheers >> Bruno Baptista >> http://twitter.com/brunobat_ <http://twitter.com/brunobat_> >> >> >> >> On 03/10/2018 18:53, Bruno Baptista wrote: >>> Thanks John and Romain, >>> >>> Will work on the new FailsafeExecutionManagerProvider. >>> >>> Cheers >>> Bruno Baptista >>> http://twitter.com/brunobat_ <http://twitter.com/brunobat_> >>> >>> >>> On 03/10/2018 18:21, John D. Ament wrote: >>>> Hi Bruno >>>> >>>> Thanks for the PR! >>>> >>>> I think my intention for what's provided in Safeguard is that we have an >>>> overideable per container integration that allows you to look up the >>>> executor. So rather than having boolean logic, you use a new >>>> implementation of FailsafeExecutionManagerProvider (perhaps as an >>>> alternative). This way the lookup can be done based on how your platform >>>> is developed. >>>> >>>> Thanks, >>>> >>>> John >>>> >>>> On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> yes, this is why I mentionned to make the pool configurable to make it >>>> work in both environment and in multiple apps with different pool. >>>> >>>> Out of my head I thought about making it injectable instead of trying all >>>> possible strategies/relying on a system properties but I just realized >>>> that we already support SE and EE with managed pool, just make a >>>> @Specializes of FailsafeExecutionManagerProvider producer. >>>> >>>> It sounds to me more flexible and easier to understand. >>>> >>>> wdyt? >>>> >>>> >>>> Romain Manni-Bucau >>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog >>>> <https://rmannibucau.metawerx.net/> | Old Blog >>>> <http://rmannibucau.wordpress.com/> | Github >>>> <https://github.com/rmannibucau> | LinkedIn >>>> <https://www.linkedin.com/in/rmannibucau> | Book >>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance> >>>> >>>> Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[email protected] >>>> <mailto:[email protected]>> a écrit : >>>> Hi Romain, >>>> >>>> I've updated the PR to get the resource location from a property. >>>> In relation to the try/catch... I'm assuming that the library is supposed >>>> to work with both SE and EE environments, if we don't catch the exception >>>> this will never work on SE. In any case, if an error exists, it will be >>>> printed and can be found. >>>> Cheers >>>> Bruno Baptista >>>> http://twitter.com/brunobat_ <http://twitter.com/brunobat_> >>>> >>>> >>>> On 03/10/2018 10:10, Romain Manni-Bucau wrote: >>>>> Hi Bruno, >>>>> >>>>> Technically this pool does not "have to be" managed ;). >>>>> >>>>> That said this is a good feature. Can you make the pool configurable >>>>> instead of hardcoding the default pool which is never used except in >>>>> tests? Will also avoid to catch and silently ignore the error (can be an >>>>> issue in servers). >>>>> >>>>> Romain Manni-Bucau >>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog >>>>> <https://rmannibucau.metawerx.net/> | Old Blog >>>>> <http://rmannibucau.wordpress.com/> | Github >>>>> <https://github.com/rmannibucau> | LinkedIn >>>>> <https://www.linkedin.com/in/rmannibucau> | Book >>>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance> >>>>> >>>>> Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[email protected] >>>>> <mailto:[email protected]>> a écrit : >>>>> Hi folks, >>>>> >>>>> Safegard creates a java se connection pool to handle the bulckhead and >>>>> async operations. When deployed in a container, that pool has to be >>>>> managed. >>>>> >>>>> I created a PR that allows to retrieve that managed pool, if available: >>>>> >>>>> https://github.com/apache/geronimo-safeguard/pull/2 >>>>> <https://github.com/apache/geronimo-safeguard/pull/2> >>>>> >>>>> Can someone please take a look at it? >>>>> >>>>> Regards >>>>> >>>>> -- >>>>> Bruno Baptista >>>>> http://twitter.com/brunobat_ <http://twitter.com/brunobat_> >>>>> >>>>> >>>> >>> >> >
