You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/04/08 09:45:31 UTC

[GitHub] [camel] TCke83 opened a new pull request #5314: CAMEL-16470 - fix for reuse EntityManager

TCke83 opened a new pull request #5314:
URL: https://github.com/apache/camel/pull/5314


   <!-- Uncomment and fill this section if your PR is not trivial
   - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it).  Trivial changes like typos do not require a JIRA issue.  Your pull request should address just this issue, without pulling in other changes.
   - [ ] Each commit in the pull request should have a meaningful subject line and body.
   - [ ] If you're unsure, you can format the pull request title like `[CAMEL-XXX] Fixes bug in camel-file component`, where you replace `CAMEL-XXX` with the appropriate JIRA issue.
   - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
   - [ ] Run `mvn clean install -Psourcecheck` in your module with source check enabled to make sure basic checks pass and there are no checkstyle violations. A more thorough check will be performed on your pull request automatically.
   Below are the contribution guidelines:
   https://github.com/apache/camel/blob/master/CONTRIBUTING.md
   -->
   


-- 
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] [camel] oscerd commented on pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
oscerd commented on pull request #5314:
URL: https://github.com/apache/camel/pull/5314#issuecomment-816662978


   Yes, it will be merged. The build is not a problem. It's one step that sometimes fail for gh actions problem. 


-- 
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] [camel] TCke83 commented on pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
TCke83 commented on pull request #5314:
URL: https://github.com/apache/camel/pull/5314#issuecomment-815626370


   I logged it as a bug, but it was changed to improvement


-- 
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] [camel] oscerd commented on pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
oscerd commented on pull request #5314:
URL: https://github.com/apache/camel/pull/5314#issuecomment-815627158


   Ah ok, it was changed by @davsclaus 
   
   I think there is no problem in backporting this PR.


-- 
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] [camel] TCke83 closed pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
TCke83 closed pull request #5314:
URL: https://github.com/apache/camel/pull/5314


   


-- 
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] [camel] oscerd commented on pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
oscerd commented on pull request #5314:
URL: https://github.com/apache/camel/pull/5314#issuecomment-816683800


   Can you please create a PR on 3.7.x too?


-- 
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] [camel] oscerd commented on pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
oscerd commented on pull request #5314:
URL: https://github.com/apache/camel/pull/5314#issuecomment-816686016


   Sorry but I had to revert. We have test failures:
   
   ```
   [ERROR] Failures: 
   [ERROR] org.apache.camel.processor.jpa.JpaRouteEndpointTest.testRouteJpa
   [ERROR]   Run 1: JpaRouteEndpointTest>JpaRouteTest.testRouteJpa:45 Assertion error at index 0 on mock mock://result with predicate: header(CamelEntityManager) instanceof javax.persistence.EntityManager on Exchange[FD16AFB6F251770-0000000000000000]
   [ERROR]   Run 2: JpaRouteEndpointTest>JpaRouteTest.testRouteJpa:45 Assertion error at index 0 on mock mock://result with predicate: header(CamelEntityManager) instanceof javax.persistence.EntityManager on Exchange[6DE3492615A862D-0000000000000000]
   [ERROR]   Run 3: JpaRouteEndpointTest>JpaRouteTest.testRouteJpa:45 Assertion error at index 0 on mock mock://result with predicate: header(CamelEntityManager) instanceof javax.persistence.EntityManager on Exchange[3F9B273D8131A0F-0000000000000000]
   [INFO] 
   [ERROR] org.apache.camel.processor.jpa.JpaRouteTest.testRouteJpa
   [ERROR]   Run 1: JpaRouteTest.testRouteJpa:45 Assertion error at index 0 on mock mock://result with predicate: header(CamelEntityManager) instanceof javax.persistence.EntityManager on Exchange[812C5CE5613B570-0000000000000000]
   [ERROR]   Run 2: JpaRouteTest.testRouteJpa:45 Assertion error at index 0 on mock mock://result with predicate: header(CamelEntityManager) instanceof javax.persistence.EntityManager on Exchange[03C7983ADDD25C4-0000000000000000]
   [ERROR]   Run 3: JpaRouteTest.testRouteJpa:45 Assertion error at index 0 on mock mock://result with predicate: header(CamelEntityManager) instanceof javax.persistence.EntityManager on Exchange[8B9C0E3B92AB9AA-0000000000000000]
   ```
   


-- 
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] [camel] oscerd commented on pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
oscerd commented on pull request #5314:
URL: https://github.com/apache/camel/pull/5314#issuecomment-816686451


   Please have a look and open a new PR.


-- 
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] [camel] vgaur commented on a change in pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
vgaur commented on a change in pull request #5314:
URL: https://github.com/apache/camel/pull/5314#discussion_r609673681



##########
File path: components/camel-jpa/src/main/java/org/apache/camel/component/jpa/JpaHelper.java
##########
@@ -54,34 +57,47 @@ public static EntityManager getTargetEntityManager(
 
         // then try reuse any entity manager which has been previously created and stored on the exchange
         if (em == null && exchange != null) {
-            em = exchange.getProperty(JpaConstants.ENTITY_MANAGER, EntityManager.class);
+            em = getEntityManagerMap(exchange).get(getKey(entityManagerFactory));
         }
 
         if (em == null && useSharedEntityManager) {

Review comment:
       since getEntityManagerMap() will now return empty map, should we add a condition for empty check as well ? 




-- 
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] [camel] oscerd commented on pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
oscerd commented on pull request #5314:
URL: https://github.com/apache/camel/pull/5314#issuecomment-815625920


   I just gave a quick look but the issue is marked as improvement and we usually don't backport improvements, even though this is safe to be backported. 


-- 
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] [camel] TCke83 commented on pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
TCke83 commented on pull request #5314:
URL: https://github.com/apache/camel/pull/5314#issuecomment-815623777


   This only allows to use different EntityManagerFactories while still allowing re-use, the behaviour is not changed when using only one EntityManagerFactory.


-- 
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] [camel] TCke83 commented on pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
TCke83 commented on pull request #5314:
URL: https://github.com/apache/camel/pull/5314#issuecomment-816661664


   @oscerd will this be merged with master? Or do I have to provide additional info? Build failed due to some strange error however.


-- 
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] [camel] oscerd merged pull request #5314: CAMEL-16470 - fix for reuse EntityManager

Posted by GitBox <gi...@apache.org>.
oscerd merged pull request #5314:
URL: https://github.com/apache/camel/pull/5314


   


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