Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-19 Thread Romain Manni-Bucau
big fingers :( thanks for the catch, should be fixed. Build in progress at https://ci.apache.org/builders/tomee-trunk-ubuntu/builds/777 Romain Manni-Bucau @rmannibucau | Blog | Old Blog

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-19 Thread Svetlin Zarev
Hi, The DataSourcePojo.java is missing from master, and the arquilian tests fail to compile. May you add it: https://github.com/apache/tomee/pull/74/commits/26ee7018df455c3a5b46c6a0b87adb38feeab34f#diff-c89f954c712eb9ab263f8acf8d75586f ? Kind regadrs, Svetlin 2017-06-17 11:13 GMT+03:00 Romain

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-17 Thread Romain Manni-Bucau
You rock Svetlin, applied, thanks a lot! Romain Manni-Bucau @rmannibucau | Blog | Old Blog | Github | LinkedIn |

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread Svetlin Zarev
The test fails, because the @DataSourceDefinition is used on a POJO - i.e. it's not a JndiConsumer, hence the ConvertDataSourceDefinitions deployer cannot know that such definition exists. I'll have to check the spec if it mentions which classes can be annotated with @DataSourceDefinition, but

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread Romain Manni-Bucau
Trunk seems to have an issue with this in the test XADataSourceDefinitionTest Want to have a look? Romain Manni-Bucau @rmannibucau | Blog | Old Blog | Github

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread Romain Manni-Bucau
applied, thanks! Romain Manni-Bucau @rmannibucau | Blog | Old Blog | Github | LinkedIn | JavaEE Factory

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread Svetlin Zarev
I've fixed the license headers. Here is the PR: https://github.com/apache/tomee/pull/73 I didn't check JMS, only WepProfile specs. 2017-06-16 21:01 GMT+03:00 Romain Manni-Bucau : > This looks good, while the merge deployer still makes available the > resource to comp it

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread Romain Manni-Bucau
This looks good, while the merge deployer still makes available the resource to comp it works :) Think you need to fix details on your PR like headers etc but looks ok to merge once the build pass. Side note: by curiosity, did you check jms resource as well? it should be close ot datasources.

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread Svetlin Zarev
I'm back with tests:) : https://github.com/apache/tomee/compare/master...SvetlinZarev:ds_def?expand=1 Unless I misunderstood you, it seems that DS injection in servlet works after the fix and does not before it (well, it injects it, but with wrong config). The reason it works is that the

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread Romain Manni-Bucau
Hmm, if you inject such a datasource in a servlet does it work? Don't think so - tests are the thing to start with when editing this code, saying that by experience if you get me ;). So concretely testing the type like that is good but it shouldn't break web component injections. Romain

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread Svetlin Zarev
Hi, I was thinking about something like https://github.com/SvetlinZarev/tomee/commit/067fd220e909e89a8c17d90122b0e2158468ece4 What do you think ? Is there a more appropriate place to check for it ? If it's OK, I can make PR with the fix + tests. Thanks, Svetlin 2017-06-16 16:15 GMT+03:00

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread Romain Manni-Bucau
Hi Svetlin this is a way to aggregate the webapp java:comp/env namespace without handling it too specifically in the code base - at least it comes from that idea. We can add it in EjbJar and just skip it at deploy time (we do something similar already, don't recally exactly where but it is typed