You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/02/14 05:26:11 UTC

[GitHub] [beam] hengfengli opened a new pull request #16845: [BEAM-12164]: allow to specify a custom metadata table for Spanner Change Streams Connector

hengfengli opened a new pull request #16845:
URL: https://github.com/apache/beam/pull/16845


   This change would allow users to specify their own custom metadata table instead of using the default auto-generated name. This would be useful for testing/development purpose. 


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] hengfengli commented on pull request #16845: [BEAM-12164]: allow to specify a custom metadata table for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
hengfengli commented on pull request #16845:
URL: https://github.com/apache/beam/pull/16845#issuecomment-1038659846


   CC @thiagotnunes 


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] pabloem merged pull request #16845: [BEAM-12164]: display the metadata table name for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
pabloem merged pull request #16845:
URL: https://github.com/apache/beam/pull/16845


   


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] hengfengli commented on pull request #16845: [BEAM-12164]: display the metadata table name for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
hengfengli commented on pull request #16845:
URL: https://github.com/apache/beam/pull/16845#issuecomment-1053927444


   As the change to allow to specify a custom metadata table is implemented in #16846, this PR is only for displaying the name of the metadata table on the Dataflow UI. 


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] hengfengli commented on pull request #16845: [BEAM-12164]: allow to specify a custom metadata table for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
hengfengli commented on pull request #16845:
URL: https://github.com/apache/beam/pull/16845#issuecomment-1053859554


   @aaltay Sorry for the delay. I have tested that the pipeline option would not pass between jobs during a hot deploy (set the pipeline option in the old job and see it in the updated job). So this PR can be merged 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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] zoercai commented on a change in pull request #16845: [BEAM-12164]: allow to specify a custom metadata table for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
zoercai commented on a change in pull request #16845:
URL: https://github.com/apache/beam/pull/16845#discussion_r805542849



##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java
##########
@@ -1496,8 +1505,10 @@ public ReadChangeStream withTraceSampleProbability(Double probability) {
               getMetadataInstance(), changeStreamDatabaseId.getInstanceId().getInstance());
       final String partitionMetadataDatabaseId =
           MoreObjects.firstNonNull(getMetadataDatabase(), changeStreamDatabaseId.getDatabase());
-      final String partitionMetadataTableName =
-          generatePartitionMetadataTableName(partitionMetadataDatabaseId);
+      String partitionMetadataTableName = getMetadataTableName();

Review comment:
       Maybe a bit simpler like the lines above?
   `MoreObjects.firstNonNull(getMetadataDatabase(), generatePartitionMetadataTableName(partitionMetadataDatabaseId));`




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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] hengfengli commented on pull request #16845: [BEAM-12164]: allow to specify a custom metadata table for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
hengfengli commented on pull request #16845:
URL: https://github.com/apache/beam/pull/16845#issuecomment-1040858441


   @pabloem Can you please take a quick review on this PR? Thanks! 


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] aaltay commented on pull request #16845: [BEAM-12164]: allow to specify a custom metadata table for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #16845:
URL: https://github.com/apache/beam/pull/16845#issuecomment-1050409044


   @hengfengli - What is the next step on 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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] hengfengli commented on a change in pull request #16845: [BEAM-12164]: allow to specify a custom metadata table for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
hengfengli commented on a change in pull request #16845:
URL: https://github.com/apache/beam/pull/16845#discussion_r806318809



##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java
##########
@@ -1496,8 +1505,10 @@ public ReadChangeStream withTraceSampleProbability(Double probability) {
               getMetadataInstance(), changeStreamDatabaseId.getInstanceId().getInstance());
       final String partitionMetadataDatabaseId =
           MoreObjects.firstNonNull(getMetadataDatabase(), changeStreamDatabaseId.getDatabase());
-      final String partitionMetadataTableName =
-          generatePartitionMetadataTableName(partitionMetadataDatabaseId);
+      String partitionMetadataTableName = getMetadataTableName();

Review comment:
       Good catch. 




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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] hengfengli commented on pull request #16845: [BEAM-12164]: display the metadata table name for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
hengfengli commented on pull request #16845:
URL: https://github.com/apache/beam/pull/16845#issuecomment-1053951573


   The failed test is unrelated to this change. 


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] pabloem commented on pull request #16845: [BEAM-12164]: display the metadata table name for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #16845:
URL: https://github.com/apache/beam/pull/16845#issuecomment-1063504626


   LGTM


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] hengfengli commented on pull request #16845: [BEAM-12164]: allow to specify a custom metadata table for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
hengfengli commented on pull request #16845:
URL: https://github.com/apache/beam/pull/16845#issuecomment-1042362644


   Please do not merge at the moment. I am investigating if we can pass the state via pipeline options between two jobs during the hot deploy. 


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] hengfengli commented on a change in pull request #16845: [BEAM-12164]: allow to specify a custom metadata table for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
hengfengli commented on a change in pull request #16845:
URL: https://github.com/apache/beam/pull/16845#discussion_r806318809



##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java
##########
@@ -1496,8 +1505,10 @@ public ReadChangeStream withTraceSampleProbability(Double probability) {
               getMetadataInstance(), changeStreamDatabaseId.getInstanceId().getInstance());
       final String partitionMetadataDatabaseId =
           MoreObjects.firstNonNull(getMetadataDatabase(), changeStreamDatabaseId.getDatabase());
-      final String partitionMetadataTableName =
-          generatePartitionMetadataTableName(partitionMetadataDatabaseId);
+      String partitionMetadataTableName = getMetadataTableName();

Review comment:
       Good catch. 




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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] hengfengli commented on pull request #16845: [BEAM-12164]: display the metadata table name for Spanner Change Streams Connector

Posted by GitBox <gi...@apache.org>.
hengfengli commented on pull request #16845:
URL: https://github.com/apache/beam/pull/16845#issuecomment-1060199170


   @pabloem Can you please take a look and help us to merge this PR? Thanks. 


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org