You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by GitBox <gi...@apache.org> on 2021/04/03 23:03:31 UTC
[GitHub] [tomee] cocorossello opened a new pull request #779: Fix some examples using BOM
cocorossello opened a new pull request #779:
URL: https://github.com/apache/tomee/pull/779
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] dblevins merged pull request #779: Fix some examples using BOM
Posted by GitBox <gi...@apache.org>.
dblevins merged pull request #779:
URL: https://github.com/apache/tomee/pull/779
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #779: Fix some examples using BOM
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #779:
URL: https://github.com/apache/tomee/pull/779#discussion_r607018168
##########
File path: examples/polling-parent/pom.xml
##########
@@ -79,29 +79,17 @@
<dependencies>
<!-- API -->
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
- <version>[8.0,)</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-api</artifactId>
- <version>${tomee.version}</version>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
<!-- impl - for standard code scope test is fine -->
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-core</artifactId>
- <version>${tomee.version}</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-cxf-rs</artifactId>
- <version>${tomee.version}</version>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
Review comment:
Should probably be `tomee-plus` instead (otherwise duplicate). Would also go for scope `test` here (as `test` would be finde for standard code but we might need to override it in submodules)
##########
File path: examples/polling-parent/polling-core/pom.xml
##########
@@ -29,20 +29,18 @@
<dependencies>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-api</artifactId>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
</dependency>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-core</artifactId>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
Review comment:
No version needed. Tree gets it for free from the parent dep mgmt section.
We have some dependencies towards `openjpa` here, so we would need `provided` here (only we have `test` in the parent pom)
##########
File path: examples/polling-parent/polling-client/pom.xml
##########
@@ -30,27 +30,9 @@
<dependencies>
<dependency>
- <groupId>org.apache.cxf</groupId>
- <artifactId>cxf-core</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.cxf</groupId>
- <artifactId>cxf-rt-rs-client</artifactId>
- <version>${cxf.version}</version>
- <exclusions>
- <exclusion>
- <groupId>javax.annotation</groupId>
- <artifactId>javax.annotation-api</artifactId>
- </exclusion>
- </exclusions>
- </dependency>
- <dependency>
- <groupId>org.apache.xbean</groupId>
- <artifactId>xbean-finder-shaded</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.xbean</groupId>
- <artifactId>xbean-reflect</artifactId>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
Review comment:
No version needed. Tree gets it for free from the parent dep mgmt section.
If you go for `test` in the parent, we need to set it to `provided` here as we have some dependencies.
##########
File path: examples/polling-parent/polling-web/pom.xml
##########
@@ -35,21 +35,14 @@
<artifactId>junit</artifactId>
</dependency>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-core</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-cxf-rs</artifactId>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
Review comment:
No version needed. Tree gets it for free from the parent dep mgmt section.
##########
File path: examples/polling-parent/pom.xml
##########
@@ -79,29 +79,17 @@
<dependencies>
<!-- API -->
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
- <version>[8.0,)</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-api</artifactId>
- <version>${tomee.version}</version>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
Review comment:
Maybe use `tomee.version` instead of the hard string as it is already defined in the `properties` section. We also can remove `cxf.version` and `xbean.version` from the `<properties` section of the parent pom.
In addition, we can remove
```
<dependency>
<groupId>org.apache.cxf</groupId>
<artifactId>cxf-core</artifactId>
<version>${cxf.version}</version>
</dependency>
<dependency>
<groupId>org.apache.xbean</groupId>
<artifactId>xbean-finder-shaded</artifactId>
<version>${xbean.version}</version>
</dependency>
<dependency>
<groupId>org.apache.xbean</groupId>
<artifactId>xbean-reflect</artifactId>
<version>${xbean.version}</version>
</dependency>
```
as it comes with the related BOM.
##########
File path: examples/polling-parent/polling-core/pom.xml
##########
@@ -29,20 +29,18 @@
<dependencies>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-api</artifactId>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
Review comment:
No version needed. Tree gets it for free from the parent dep mgmt section.
##########
File path: examples/applicationcomposer-jaxws-cdi/pom.xml
##########
@@ -64,10 +64,10 @@
</repositories>
<dependencies>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
- <version>[8.0,)</version>
- <scope>provided</scope>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
+ <scope>test</scope>
Review comment:
Shouldn't it be `provided` ?
##########
File path: examples/polling-parent/polling-client/pom.xml
##########
@@ -62,8 +44,9 @@
<version>${project.version}</version>
</dependency>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
Review comment:
No version needed. Tree gets it for free from the parent dep mgmt section.
##########
File path: examples/polling-parent/polling-domain/pom.xml
##########
@@ -29,8 +29,9 @@
<dependencies>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
Review comment:
No version needed. Tree gets it for free from the parent dep mgmt section.
##########
File path: examples/applicationcomposer-jaxws-cdi/pom.xml
##########
@@ -64,10 +64,10 @@
</repositories>
<dependencies>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
- <version>[8.0,)</version>
- <scope>provided</scope>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
+ <scope>test</scope>
Review comment:
Otherwise:
It seems, that the tests are failing as the `tomee-plus` boms brings in `tomee-security`, which expects some servlet registrations in `TomEESecurityServletAuthenticationMechanismMapper` and subsequently fails with a `NullPointerException`.
So either
- (a) The test is flawed and requires some additional parameters / registrations
- (b) `TomEESecurityServletAuthenticationMechanismMapper` requires a ` != null` check on `servletRegistrations` in `init(...)`
- (c) Exclude `tomee-security` in the examples dependencies
- (d) Something else (disable security in the example, other things, ...) ....
Maybe @dblevins has some thoughts here?
##########
File path: examples/polling-parent/polling-web/pom.xml
##########
@@ -35,21 +35,14 @@
<artifactId>junit</artifactId>
</dependency>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-core</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>openejb-cxf-rs</artifactId>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
Review comment:
In addition: `provided` if `test` in parent pom.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] dblevins commented on pull request #779: Fix some examples using BOM
Posted by GitBox <gi...@apache.org>.
dblevins commented on pull request #779:
URL: https://github.com/apache/tomee/pull/779#issuecomment-814559280
Not sure why Jenkins didn't build this PR after it was updated, but merging to see if we can fix the build.
Thank you @cocorossello
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] cocorossello commented on pull request #779: Fix some examples using BOM
Posted by GitBox <gi...@apache.org>.
cocorossello commented on pull request #779:
URL: https://github.com/apache/tomee/pull/779#issuecomment-812988072
Still not working:
examples/polling-parent/polling-web/pom.xml
examples/applicationcomposer-jaxws-cdi/pom.xml
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] cocorossello commented on a change in pull request #779: Fix some examples using BOM
Posted by GitBox <gi...@apache.org>.
cocorossello commented on a change in pull request #779:
URL: https://github.com/apache/tomee/pull/779#discussion_r607245206
##########
File path: examples/applicationcomposer-jaxws-cdi/pom.xml
##########
@@ -64,10 +64,10 @@
</repositories>
<dependencies>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
- <version>[8.0,)</version>
- <scope>provided</scope>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
+ <scope>test</scope>
Review comment:
Excluded, thanks, I didn't know whether servletRegistrations should be not null or (more likely) servletRegistrations should not be called
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on pull request #779: Fix some examples using BOM
Posted by GitBox <gi...@apache.org>.
rzo1 commented on pull request #779:
URL: https://github.com/apache/tomee/pull/779#issuecomment-814626199
I opened a PR to test the integration a few days ago, which worked. @cocorossello uses a shared repository for the PRs - might be the reason why Jenkins does not pick up these PRs - would be imho an odd behaviour of the Jenkins Plugin.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #779: Fix some examples using BOM
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #779:
URL: https://github.com/apache/tomee/pull/779#discussion_r607288878
##########
File path: examples/applicationcomposer-jaxws-cdi/pom.xml
##########
@@ -64,10 +64,10 @@
</repositories>
<dependencies>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
- <version>[8.0,)</version>
- <scope>provided</scope>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
+ <scope>test</scope>
Review comment:
@cocorossello The `api` module needs to be in scope `provided` in this example. Besides that: Works fine for me locally :) (and all other examples are passing the tests too... so overall lgtm. thx!)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] dblevins commented on a change in pull request #779: Fix some examples using BOM
Posted by GitBox <gi...@apache.org>.
dblevins commented on a change in pull request #779:
URL: https://github.com/apache/tomee/pull/779#discussion_r607192663
##########
File path: examples/applicationcomposer-jaxws-cdi/pom.xml
##########
@@ -64,10 +64,10 @@
</repositories>
<dependencies>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
- <version>[8.0,)</version>
- <scope>provided</scope>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
+ <scope>test</scope>
Review comment:
My recommendation would be to for both C and B. Specifically, do the exclude (B) in this PR so we can get the build working, then execute C in a different PR and remove the exclude.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tomee] dblevins commented on a change in pull request #779: Fix some examples using BOM
Posted by GitBox <gi...@apache.org>.
dblevins commented on a change in pull request #779:
URL: https://github.com/apache/tomee/pull/779#discussion_r607192663
##########
File path: examples/applicationcomposer-jaxws-cdi/pom.xml
##########
@@ -64,10 +64,10 @@
</repositories>
<dependencies>
<dependency>
- <groupId>org.apache.tomee</groupId>
- <artifactId>javaee-api</artifactId>
- <version>[8.0,)</version>
- <scope>provided</scope>
+ <groupId>org.apache.tomee.bom</groupId>
+ <artifactId>tomee-plus-api</artifactId>
+ <version>8.0.7-SNAPSHOT</version>
+ <scope>test</scope>
Review comment:
My recommendation would be to for both C and B. Specifically, do the exclude (B) so we can get the build working, then execute C and remove the exclude.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org