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/03/28 21:26:01 UTC

[GitHub] [tomee] cocorossello opened a new pull request #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencias

cocorossello opened a new pull request #777:
URL: https://github.com/apache/tomee/pull/777


   


-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
dblevins commented on a change in pull request #777:
URL: https://github.com/apache/tomee/pull/777#discussion_r603689991



##########
File path: server/openejb-cxf/pom.xml
##########
@@ -191,7 +195,7 @@
           <artifactId>jaxb-xjc</artifactId>
         </exclusion>
         <exclusion>
-          <groupId>net.sf.ehcache</groupId>
+          <groupId>org.ehcache</groupId>

Review comment:
       Agree that if CXF upgraded from `ehcache` v2 to v3, we should do the work to ensure that's the one we're using in the pomx and update our exclusion lists as/if applicable for the new groupId.




-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
dblevins commented on pull request #777:
URL: https://github.com/apache/tomee/pull/777#issuecomment-808967604


   @cocorossello Thanks for the upgrades!  Would you be willing to run a TCK test or two to verify all is good after the changes?
   
   Here are the setup instructions:
   
    - https://github.com/apache/tomee-tck#tck-setup
   
   This is a short little section of 20 tests that should be good enough to smoke test:
   
    -  `./runtests -c --web tomee-plume com.sun.ts.tests.jaxrs.api.rs.core.mediatype`
    


-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
dblevins commented on pull request #777:
URL: https://github.com/apache/tomee/pull/777#issuecomment-812635140


   The results look great!  This actually fixed 7 tests in the JAX-RS section of the TCK.  One test appears to have regressed, but it could be an intermittent failure.
   
    - https://tck.work/tomee/tests?build=1617332730829&path=com.sun.ts.tests.jaxrs&compare=1617155474930
   
   Thank you so much, @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] dblevins commented on pull request #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
dblevins commented on pull request #777:
URL: https://github.com/apache/tomee/pull/777#issuecomment-812681094


   Yeah, I have some thoughts there.  I'll post to the list so we can clean it up for good (hopefully)


-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #777:
URL: https://github.com/apache/tomee/pull/777#discussion_r603849824



##########
File path: server/openejb-cxf/pom.xml
##########
@@ -191,7 +195,7 @@
           <artifactId>jaxb-xjc</artifactId>
         </exclusion>
         <exclusion>
-          <groupId>net.sf.ehcache</groupId>
+          <groupId>org.ehcache</groupId>

Review comment:
       It seems, that the V2 dependency (defined in the root pom) is only used in `openejb-core-hibernate` module, which is correct as `hibernate-ehcache` is still using V2, see [dependency:tree](https://gist.github.com/rzo1/86d87a50bce0e1f8c41f87edc0618a17).
   
   Nevertheless, it might be worth adding `org.ehcache` to the exclusion lists in `ContainerClassesFilter` and `JuliLogStreamFactory` (to be future safe...). Besides that, we are probably good here.




-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #777:
URL: https://github.com/apache/tomee/pull/777#discussion_r603250400



##########
File path: server/openejb-cxf/pom.xml
##########
@@ -191,7 +195,7 @@
           <artifactId>jaxb-xjc</artifactId>
         </exclusion>
         <exclusion>
-          <groupId>net.sf.ehcache</groupId>
+          <groupId>org.ehcache</groupId>

Review comment:
       Good catch. Seems they upgrade `ehcache` from v2 to v3 with 3.4.x
   
   Don't know, if this affects anything in TomEE itself as we still have `ehcache` v2 in the classpath (at least we declare it in the root pom). Consequently, I am wondering, if this will be an problem, if we exclude the v3 `ehcache` here`. 
   
   Might be also relevant for other libs, which come with v3 (such as `wss4j-ws-security-stax` - exclude on `ǹet.sf` doesnt work anymore...). Consequently, I am wondering, if we need to upgrade the `ehcache` version in TomEE as well (would require to add  `org.ehcache` to the exclusion lists in `ContainerClassesFilter` and `JuliLogStreamFactory`).
   
   Maybe @dblevins has some thoughts on it?




-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencias

Posted by GitBox <gi...@apache.org>.
cocorossello commented on a change in pull request #777:
URL: https://github.com/apache/tomee/pull/777#discussion_r602936639



##########
File path: server/openejb-cxf-transport/src/main/java/org/apache/openejb/server/cxf/transport/util/CxfUtil.java
##########
@@ -315,7 +315,6 @@ public static void configureBus() {
                 final InstrumentationManagerImpl manager = InstrumentationManagerImpl.class.cast(mgr);
                 manager.setEnabled(true);
                 manager.setServer(LocalMBeanServer.get());
-                manager.setDaemon(true);

Review comment:
       I guess it was removed in https://github.com/apache/cxf/commit/1cf4fed546904a4a2560f53a2a2391d834b4026c#diff-06b277ed5b1de60d05718de5e55b47a9b0dcaf25360df46fbdaf1978fe70c30f




-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #777:
URL: https://github.com/apache/tomee/pull/777#discussion_r603250400



##########
File path: server/openejb-cxf/pom.xml
##########
@@ -191,7 +195,7 @@
           <artifactId>jaxb-xjc</artifactId>
         </exclusion>
         <exclusion>
-          <groupId>net.sf.ehcache</groupId>
+          <groupId>org.ehcache</groupId>

Review comment:
       Good catch. Seems they upgraded `ehcache` from v2 to v3 with 3.4.x
   
   Don't know, if this affects anything in TomEE itself as we still have `ehcache` v2 in the classpath (at least we declare it in the root pom). Consequently, I am wondering, if this will be an problem, if we exclude the v3 `ehcache` here`. 
   
   Might be also relevant for other libs, which come with v3 (such as `wss4j-ws-security-stax` - exclude on `ǹet.sf` doesnt work anymore...). Consequently, I am wondering, if we need to upgrade the `ehcache` version in TomEE as well (would require to add  `org.ehcache` to the exclusion lists in `ContainerClassesFilter` and `JuliLogStreamFactory`).
   
   Maybe @dblevins has some thoughts on it?

##########
File path: server/openejb-cxf/pom.xml
##########
@@ -191,7 +195,7 @@
           <artifactId>jaxb-xjc</artifactId>
         </exclusion>
         <exclusion>
-          <groupId>net.sf.ehcache</groupId>
+          <groupId>org.ehcache</groupId>

Review comment:
       Might be interesting to do some `mvn dependency:tree` analysis related to ehcache? :)




-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencias

Posted by GitBox <gi...@apache.org>.
dblevins commented on a change in pull request #777:
URL: https://github.com/apache/tomee/pull/777#discussion_r602935304



##########
File path: server/openejb-cxf-transport/src/main/java/org/apache/openejb/server/cxf/transport/util/CxfUtil.java
##########
@@ -315,7 +315,6 @@ public static void configureBus() {
                 final InstrumentationManagerImpl manager = InstrumentationManagerImpl.class.cast(mgr);
                 manager.setEnabled(true);
                 manager.setServer(LocalMBeanServer.get());
-                manager.setDaemon(true);

Review comment:
       Does this call no longer exist in the latest CXF version?




-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #777:
URL: https://github.com/apache/tomee/pull/777#discussion_r603250400



##########
File path: server/openejb-cxf/pom.xml
##########
@@ -191,7 +195,7 @@
           <artifactId>jaxb-xjc</artifactId>
         </exclusion>
         <exclusion>
-          <groupId>net.sf.ehcache</groupId>
+          <groupId>org.ehcache</groupId>

Review comment:
       Good catch. Seems they upgrade `ehcache` from v2 to v3 with 3.4.x
   
   Don't know, if this affects anything in TomEE itself as we are still have `ehcache` v2 in the classpath. Consequently, I am wondering, if this will be an problem, if we exclude the v3 `ehcache` here`. 
   
   Might be also relevant for other libs, which come with v3 (such as `wss4j-ws-security-stax` - exclude on `ǹet.sf` doesnt work anymore...). Consequently, I am wondering, if we need to upgrade the `ehcache` version in TomEE as well (would require to add  `org.ehcache` to the exclusion lists in `ContainerClassesFilter` and `JuliLogStreamFactory`).
   
   Maybe @dblevins have some thoughts on it?




-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
rzo1 commented on pull request #777:
URL: https://github.com/apache/tomee/pull/777#issuecomment-812675568


   We also have some failing tests (examples) on the master build related to the upgrade: `NoSuchMethodError` - I guess most of them should be easily fixable :)


-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
dblevins commented on pull request #777:
URL: https://github.com/apache/tomee/pull/777#issuecomment-812270887


   Merging and will kick off a full TCK run to see how it goes.  Thank you @cocorossello for the patch and @rzo1 for the careful review.


-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #777:
URL: https://github.com/apache/tomee/pull/777#discussion_r603250400



##########
File path: server/openejb-cxf/pom.xml
##########
@@ -191,7 +195,7 @@
           <artifactId>jaxb-xjc</artifactId>
         </exclusion>
         <exclusion>
-          <groupId>net.sf.ehcache</groupId>
+          <groupId>org.ehcache</groupId>

Review comment:
       Good catch. Seems they upgrade `ehcache` from v2 to v3 with 3.4.x
   
   Don't know, if this affects anything in TomEE itself as we are still have `ehcache` v2 in the classpath. Consequently, I am wondering, if this will be an problem, if we exclude the v3 `ehcache` here`. 
   
   Might be also relevant for other libs, which come with v3 (such as `wss4j-ws-security-stax` - exclude on `ǹet.sf` doesnt work anymore...). Consequently, I am wondering, if we need to upgrade the `ehcache` version in TomEE as well (would require to add  `org.ehcache` to the exclusion lists in `ContainerClassesFilter` and `JuliLogStreamFactory`).
   
   Maybe @dblevins has some thoughts on it?




-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
dblevins commented on a change in pull request #777:
URL: https://github.com/apache/tomee/pull/777#discussion_r603689991



##########
File path: server/openejb-cxf/pom.xml
##########
@@ -191,7 +195,7 @@
           <artifactId>jaxb-xjc</artifactId>
         </exclusion>
         <exclusion>
-          <groupId>net.sf.ehcache</groupId>
+          <groupId>org.ehcache</groupId>

Review comment:
       Agree that if CXF upgraded from `ehcache` v2 to v3, we should do the work to ensure that's the one we're using in the poms and update our exclusion lists as/if applicable for the new groupId.




-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
dblevins merged pull request #777:
URL: https://github.com/apache/tomee/pull/777


   


-- 
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 #777: TOMEE-2987: Upgrade CXF to latest 3.4.3 and align dependencies

Posted by GitBox <gi...@apache.org>.
cocorossello commented on a change in pull request #777:
URL: https://github.com/apache/tomee/pull/777#discussion_r603533959



##########
File path: server/openejb-cxf/pom.xml
##########
@@ -191,7 +195,7 @@
           <artifactId>jaxb-xjc</artifactId>
         </exclusion>
         <exclusion>
-          <groupId>net.sf.ehcache</groupId>
+          <groupId>org.ehcache</groupId>

Review comment:
       I did this looking at the lib folder in the generated tomee, didn't see the old ehcache there, but yes, maybe it's better to also upgrade as well, don't really know for sure.




-- 
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