Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-28 Thread Jonathan Gallimore
Even after putting back SystemInstance.get().setProperty("openejb.cxf-rs.cache-application", "false");, this still has multiple failures across health, metrics and OpenAPI. I'll retest it again, but I'd be grateful for another pair of eyes. Thanks Jon On Tue, May 28, 2019 at 2:52 PM Jonathan Gal

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-28 Thread Jonathan Gallimore
Looking back at this one. I reverted the removal of the system property - I think we can address this in another PR. Hoping I can merge this one shortly. Jon On Fri, May 17, 2019 at 12:33 PM Ivan Junckes Filho wrote: > Hi Jon, I understood differently. I thought it was green and ready to be > m

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-17 Thread Ivan Junckes Filho
Hi Jon, I understood differently. I thought it was green and ready to be merged that is why I pushed. Sorry about that! No hurry, then. I will see if I can get some time to look into this. On Thu, May 16, 2019 at 10:39 AM Jonathan Gallimore < jonathan.gallim...@gmail.com> wrote: > Currently th

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-16 Thread Jonathan Gallimore
Currently there's a handful of tests that fail with this PR. If you're able to help unpick the issues, that would be great. Jon On Thu, May 16, 2019 at 1:48 PM Ivan Junckes Filho wrote: > Awesome, Jon. Thanks a lot for making the test. > > Would be awesome if we can get this in M3. > > On Thu,

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-16 Thread Ivan Junckes Filho
Awesome, Jon. Thanks a lot for making the test. Would be awesome if we can get this in M3. On Thu, May 16, 2019 at 9:31 AM Jonathan Gallimore < jonathan.gallim...@gmail.com> wrote: > I built a test: https://github.com/apache/tomee/pull/467, and tried the PR > with > the SystemInstance.get().setP

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-16 Thread Jonathan Gallimore
I built a test: https://github.com/apache/tomee/pull/467, and tried the PR with the SystemInstance.get().setProperty("openejb.cxf-rs.cache-application", "false"); line removed. The PR fixes the test (great). Removing the property looks like it potentially has other impacts - at least one test in Op

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-15 Thread Jonathan Gallimore
Thanks. That helps a lot. Definitely different to what we're seeing with the .war file that the TCK uses. I'll take a look at that with a debugger. Jon On Wed, May 15, 2019 at 1:58 PM Ivan Junckes Filho wrote: > Jon, thank you for helping out with this. > > I created a sample project with the d

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-15 Thread Ivan Junckes Filho
Jon, thank you for helping out with this. I created a sample project with the description of the issue and how to reproduce it. See here: https://github.com/ivanjunckes/openapi_sample On Wed, May 15, 2019 at 6:54 AM Jonathan Gallimore < jonathan.gallim...@gmail.com> wrote: > I built master (wit

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-15 Thread Jonathan Gallimore
I built master (without this PR), and deployed the .war file that the openapi-tck creates, which includes this class: https://github.com/eclipse/microprofile-open-api/blob/master/tck/src/main/java/org/eclipse/microprofile/openapi/apps/airlines/JAXRSApp.java (.war file attached - not sure if it'll m

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-14 Thread David Blevins
> On May 15, 2019, at 2:55 AM, Ivan Junckes Filho wrote: > > Adding openejb.cxf-rs.cache-application=false to system.properties in M2 > doesn't fix the issue. That's refreshing and not surprising. I'm not a fan of these kinds of system properties. The theme of them is often that we couldn't f

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-14 Thread Jonathan Gallimore
I'll see if I can help you add a test so we have a better understanding of what's going on, and help get this PR over the line. Jon On Tue, 14 May 2019, 18:56 Ivan Junckes Filho, wrote: > David, I will try to create a good description of the problem tomorrow > morning. > > Adding openejb.cxf-rs

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-14 Thread Ivan Junckes Filho
David, I will try to create a good description of the problem tomorrow morning. Adding openejb.cxf-rs.cache-application=false to system.properties in M2 doesn't fix the issue. On Tue, May 14, 2019 at 1:37 PM Roberto Cortez wrote: > It is. We added it here: > > > https://github.com/apache/tome

Re: TomEE Important PR pending - Merge please?

2019-05-14 Thread Roberto Cortez
Yes, that flag was added there on purpose to enable OpenAPI to reach the real Application and not the wrapper classed created by TomEE. That seemed to be enough to pass the TCK. Now if there are other cases that are not working, we need to figure out why exactly. Cheers, Roberto > On 14 May 2

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-14 Thread Roberto Cortez
It is. We added it here: https://github.com/apache/tomee/blob/a21d73afca6d7842333a5fb0e0c1a3eff653f983/tomee/tomee-microprofile/mp-common/src/main/java/org/apache/tomee/microprofile/TomEEMicroProfileListener.java#L93

Re: TomEE Important PR pending - Merge please?

2019-05-14 Thread Jonathan Gallimore
I think the PR should include the removal of that flag. I'm guessing that this is something to do with it: https://github.com/apache/tomee/blob/master/tomee/tomee-microprofile/mp-common/src/main/java/org/apache/tomee/microprofile/TomEEMicroProfileListener.java#L93 . That's in production code thoug

Re: @OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-14 Thread David Blevins
> On May 15, 2019, at 12:55 AM, David Blevins wrote: > > Side note to everyone on TCKs and system properties: we have to pass TCKs > with the default settings we ship. If we have to disable/enable features to > pass a TCK, we haven't passed the TCK. On this topic: - what is the flag we enab

@OpenAPIDefinition not working (was Re: TomEE Important PR pending - Merge please?)

2019-05-14 Thread David Blevins
Hey all, I will happily donate my time to do another milestone next week if we can focus on doing the right thing now. I see three forms of technical debt in this exchange: - No tests - The only real description is a long email thread - An undocumented property no one understands Side note

Re: TomEE Important PR pending - Merge please?

2019-05-14 Thread Ivan Junckes Filho
That is the goal of the PR. On Tue, May 14, 2019 at 12:40 PM Jonathan Gallimore < jonathan.gallim...@gmail.com> wrote: > > Removing that flag doesn't fix the problem unfortunately. > > With this PR, the TCK (which is running with a flag) should now pass > without the flag, is that correct? > > On

Re: TomEE Important PR pending - Merge please?

2019-05-14 Thread Jonathan Gallimore
> Removing that flag doesn't fix the problem unfortunately. With this PR, the TCK (which is running with a flag) should now pass without the flag, is that correct? On Tue, May 14, 2019 at 3:38 PM Ivan Junckes Filho wrote: > Removing that flag doesn't fix the problem unfortunately. I saw your >

Re: TomEE Important PR pending - Merge please?

2019-05-14 Thread Ivan Junckes Filho
Removing that flag doesn't fix the problem unfortunately. I saw your comment Jon, but the issue is not with Geronimo as far as I understand. TomEE uses InternalApplication instead of using the custom Application config and because of that geronimo doesn't pick it up in the geronimo extension. I ad

Re: TomEE Important PR pending - Merge please?

2019-05-14 Thread Jean-Louis Monteiro
Agreed. Le mar. 14 mai 2019 à 15:24, Jonathan Gallimore < jonathan.gallim...@gmail.com> a écrit : > I added a note on the PR. I did find the discussion on the mailing list: > > http://tomee-openejb.979440.n4.nabble.com/OpenAPIDefinition-not-working-td4687918.html > . > > It sounded like a flag is

Re: TomEE Important PR pending - Merge please?

2019-05-14 Thread Jonathan Gallimore
I added a note on the PR. I did find the discussion on the mailing list: http://tomee-openejb.979440.n4.nabble.com/OpenAPIDefinition-not-working-td4687918.html . It sounded like a flag is needed to pass the TCK currently - preference would be that there is a test for this issue (which could just b

Re: TomEE Important PR pending - Merge please?

2019-05-14 Thread Jean-Louis Monteiro
Hey Ivan. I had a look but as is. It can't be merge. At least the apache header needs to be added. Can you contribute to it? Also there is no test even happy path. So not sure what it fixes. Le mar. 14 mai 2019 à 14:29, Ivan Junckes Filho a écrit : > Hey guys, this PR from Otavio fixes a very b

TomEE Important PR pending - Merge please?

2019-05-14 Thread Ivan Junckes Filho
Hey guys, this PR from Otavio fixes a very bad issue with OpenAPI in TomEE regarding the use of InternalApplication. https://issues.apache.org/jira/browse/TOMEE-2502 I reviewed the PR and it really fixes the issue. Can someone please merge this? This is pending for awhile and I would appreciate