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 2020/10/01 20:49:19 UTC

[GitHub] [camel] alvinkwekel opened a new pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

alvinkwekel opened a new pull request #4343:
URL: https://github.com/apache/camel/pull/4343


   When Camel receives a PubSub message with reserved Google PubSub attributes these are set on a Camel header and passed on to any message sender. These reserved attributes are not allowed to be set on an outgoing message so this throws an error.
   
   This filters out these reserved attributes before sending.
   
   https://issues.apache.org/jira/browse/CAMEL-15617


----------------------------------------------------------------
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] alvinkwekel commented on a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: components/camel-google-pubsub/pom.xml
##########
@@ -47,83 +47,7 @@
             <groupId>com.google.cloud</groupId>
             <artifactId>google-cloud-pubsub</artifactId>
             <version>${google-cloud-pubsub-version}</version>
-            <exclusions>

Review comment:
       Sorry, GRPC 1.30.2. So that would mean a general _upgrade_. Would that be possible?




----------------------------------------------------------------
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] alvinkwekel commented on pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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


   Perhaps its better to keep Google PubSub at 1.105.0 for now because that uses the same GRPC version as the Camel GRPC component.


----------------------------------------------------------------
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] alvinkwekel commented on pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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


   Yes, all tests pass when run against the Cloud PubSub. Although I did notice exceptions being thrown when running the suite: about GRPC channels not being shutdown properly. So I added an additional fix to shutdown all cached producers when the component is shutdown. This solves these exceptions.


----------------------------------------------------------------
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] alvinkwekel commented on a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: components/camel-google-pubsub/pom.xml
##########
@@ -47,83 +47,7 @@
             <groupId>com.google.cloud</groupId>
             <artifactId>google-cloud-pubsub</artifactId>
             <version>${google-cloud-pubsub-version}</version>
-            <exclusions>

Review comment:
       PubSub 1.108.1 has a dependency on GRPC 1.57.1 and it doesn't work with the GRPC 1.28.0 as defined in the parent.
   Should I increase the version in the parent, add a pubsub specific version in the parent of leave it like so?




----------------------------------------------------------------
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 a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: components/camel-google-pubsub/pom.xml
##########
@@ -47,83 +47,7 @@
             <groupId>com.google.cloud</groupId>
             <artifactId>google-cloud-pubsub</artifactId>
             <version>${google-cloud-pubsub-version}</version>
-            <exclusions>

Review comment:
       I don't think camel-grpc is ready for this. So I believe it would be better to use a specific version only in this component 




----------------------------------------------------------------
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] alvinkwekel commented on a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: parent/pom.xml
##########
@@ -215,7 +215,7 @@
         <google-api-services-mail-version>v1-rev81-1.22.0</google-api-services-mail-version>
         <google-api-services-bigquery-version>v2-rev352-1.22.0</google-api-services-bigquery-version>
         <google-api-services-pubsub-version>v1-rev12-1.22.0</google-api-services-pubsub-version>
-        <google-cloud-pubsub-version>1.102.0</google-cloud-pubsub-version>
+        <google-cloud-pubsub-version>1.108.1</google-cloud-pubsub-version>

Review comment:
       Yes that is correct, see my comment above.




----------------------------------------------------------------
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] alvinkwekel commented on a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: components/camel-google-pubsub/pom.xml
##########
@@ -47,83 +47,7 @@
             <groupId>com.google.cloud</groupId>
             <artifactId>google-cloud-pubsub</artifactId>
             <version>${google-cloud-pubsub-version}</version>
-            <exclusions>

Review comment:
       I've created an explicit dependency on GRPC 1.30.2 now.
   
   




----------------------------------------------------------------
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] davsclaus merged pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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


   


----------------------------------------------------------------
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] alvinkwekel commented on a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: components/camel-google-pubsub/pom.xml
##########
@@ -47,83 +47,7 @@
             <groupId>com.google.cloud</groupId>
             <artifactId>google-cloud-pubsub</artifactId>
             <version>${google-cloud-pubsub-version}</version>
-            <exclusions>

Review comment:
       PubSub 1.108.1 has a dependency on GRPC 1.30.2 and it doesn't work with the GRPC 1.28.0 as defined in the parent.
   Should I increase the version in the parent, add a pubsub specific version in the parent of leave it like so?




----------------------------------------------------------------
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 a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: parent/pom.xml
##########
@@ -205,7 +205,7 @@
         <geronimo-ws-metadata-spec-version>1.1.3</geronimo-ws-metadata-spec-version>
         <gmetric4j-version>1.0.10</gmetric4j-version>
         <google-guava-version>19.0</google-guava-version>
-        <google-pubsub-guava-version>28.2-jre</google-pubsub-guava-version>
+        <google-pubsub-guava-version>29.0-jre</google-pubsub-guava-version>

Review comment:
       We need to check the camel-googke-pubsub karaf feature in the camel-karaf repo, but we can do that later




----------------------------------------------------------------
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 a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: parent/pom.xml
##########
@@ -205,7 +205,7 @@
         <geronimo-ws-metadata-spec-version>1.1.3</geronimo-ws-metadata-spec-version>
         <gmetric4j-version>1.0.10</gmetric4j-version>
         <google-guava-version>19.0</google-guava-version>
-        <google-pubsub-guava-version>28.2-jre</google-pubsub-guava-version>
+        <google-pubsub-guava-version>29.0-jre</google-pubsub-guava-version>

Review comment:
       I think this require a check on the karaf feature if needed

##########
File path: components/camel-google-pubsub/pom.xml
##########
@@ -47,83 +47,7 @@
             <groupId>com.google.cloud</groupId>
             <artifactId>google-cloud-pubsub</artifactId>
             <version>${google-cloud-pubsub-version}</version>
-            <exclusions>

Review comment:
       Are you sure about removing the exclusions?

##########
File path: parent/pom.xml
##########
@@ -215,7 +215,7 @@
         <google-api-services-mail-version>v1-rev81-1.22.0</google-api-services-mail-version>
         <google-api-services-bigquery-version>v2-rev352-1.22.0</google-api-services-bigquery-version>
         <google-api-services-pubsub-version>v1-rev12-1.22.0</google-api-services-pubsub-version>
-        <google-cloud-pubsub-version>1.102.0</google-cloud-pubsub-version>
+        <google-cloud-pubsub-version>1.108.1</google-cloud-pubsub-version>

Review comment:
       I think that's the reason for removing the exclusions, but we need to double check




----------------------------------------------------------------
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] alvinkwekel commented on a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: components/camel-google-pubsub/pom.xml
##########
@@ -47,83 +47,7 @@
             <groupId>com.google.cloud</groupId>
             <artifactId>google-cloud-pubsub</artifactId>
             <version>${google-cloud-pubsub-version}</version>
-            <exclusions>

Review comment:
       Sorry, GRPC 1.30.2. So that would mean a general upgrade. Would that be possible?




----------------------------------------------------------------
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] alvinkwekel commented on a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: parent/pom.xml
##########
@@ -205,7 +205,7 @@
         <geronimo-ws-metadata-spec-version>1.1.3</geronimo-ws-metadata-spec-version>
         <gmetric4j-version>1.0.10</gmetric4j-version>
         <google-guava-version>19.0</google-guava-version>
-        <google-pubsub-guava-version>28.2-jre</google-pubsub-guava-version>
+        <google-pubsub-guava-version>29.0-jre</google-pubsub-guava-version>

Review comment:
       You mean in the camel-dependencies project right? I've updated it there now.




----------------------------------------------------------------
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 a change in pull request #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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



##########
File path: components/camel-google-pubsub/pom.xml
##########
@@ -47,83 +47,7 @@
             <groupId>com.google.cloud</groupId>
             <artifactId>google-cloud-pubsub</artifactId>
             <version>${google-cloud-pubsub-version}</version>
-            <exclusions>

Review comment:
       Do you mean 1.27.1? I think it makes sense to add a property placeholder only for pubsub and use it in this component only. We can do the same for the karaf feature. Camel-grpc is based on 1.28.0 so we cannot generally downgrade




----------------------------------------------------------------
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 #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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


   Thanks, I'll merge this today.


----------------------------------------------------------------
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 #4343: CAMEL-15617: Fix filter Google PubSub reserved attributes

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


   Let me wait for the answer before merging then. 


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