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 2020/05/08 22:12:12 UTC

[GitHub] [beam] mf2199 opened a new pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

mf2199 opened a new pull request #11295:
URL: https://github.com/apache/beam/pull/11295


   [CBT connector: Expanding `bigtableio` with `read` functionality.]
   
   **Please** add a meaningful description for your change here
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
   --- | --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
   Java | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
   Python | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
   XLang | --- | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) 
   Portable | --- | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   


----------------------------------------------------------------
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] [beam] chamikaramj commented on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

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


   Lots were not available anymore. Re-triggerred.


----------------------------------------------------------------
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] [beam] chamikaramj commented on pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

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


   Run Python PreCommit


----------------------------------------------------------------
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] [beam] chamikaramj commented on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

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


   Retest this please


----------------------------------------------------------------
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] [beam] chamikaramj edited a comment on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

Posted by GitBox <gi...@apache.org>.
chamikaramj edited a comment on pull request #11295:
URL: https://github.com/apache/beam/pull/11295#issuecomment-662555479


   Logs were not available anymore. Re-triggerred.


----------------------------------------------------------------
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] [beam] aaltay commented on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

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


   retest this please


----------------------------------------------------------------
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] [beam] stale[bot] closed pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #11295:
URL: https://github.com/apache/beam/pull/11295


   


----------------------------------------------------------------
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] [beam] chamikaramj commented on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

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


   IIRC we could not get BT integration tests working with this PR. Also I think some folks from Google side was going to improve this or look into this but not sure if what work happened. I would like to get this PR to a good working state (including working ITs) before submitting. I think a getting a connector that is not fully fleshed out submitted will do more harm than good.
   
   Another option will be to add a multi-language wrapper for already established Java BT connector which should work for portable runners and Dataflow Runner v2.


-- 
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] [beam] aaltay commented on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

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


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

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



[GitHub] [beam] chamikaramj commented on pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

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






----------------------------------------------------------------
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] [beam] chamikaramj commented on pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

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


   Run Python PostCommit


----------------------------------------------------------------
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] [beam] chamikaramj commented on pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

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


   Please ping here if any tests have to be re-triggered.


----------------------------------------------------------------
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] [beam] chamikaramj commented on pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

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


   Can you please move these modules (including the BigTable sink) to io/gcp/experimental.
   
   We should leave this there till we have have proper integration and performance tests in place to be confident about the production readiness of this the BigTable source and sink.


----------------------------------------------------------------
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] [beam] stale[bot] commented on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #11295:
URL: https://github.com/apache/beam/pull/11295#issuecomment-743934679


   This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
   


----------------------------------------------------------------
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] [beam] mf2199 edited a comment on pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

Posted by GitBox <gi...@apache.org>.
mf2199 edited a comment on pull request #11295:
URL: https://github.com/apache/beam/pull/11295#issuecomment-640277483


   @chamikaramj Could you please re-trigger the tests.


----------------------------------------------------------------
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] [beam] stale[bot] commented on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #11295:
URL: https://github.com/apache/beam/pull/11295#issuecomment-751236035


   This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
   


----------------------------------------------------------------
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] [beam] mf2199 commented on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

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


   @chamikaramj @aaltay At this point, the errors point to missing arguments that were made so by design, e.g.:
   ```
   15:41:25 E       ValueError: Pipeline has validations errors: 
   15:41:25 E       Missing required option: project.
   15:41:25 E       Missing required option: region.
   15:41:25 E       Missing GCS path option: temp_location.
   15:41:25 E       Missing GCS path option: staging_location.
   ```
   [from "Run Python PreCommit" log]
   
   These were left out purposely, as they may contain non-public information and subject to change. 
   
   Any ideas?


----------------------------------------------------------------
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] [beam] chamikaramj commented on pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

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


   Retest this please


----------------------------------------------------------------
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] [beam] mf2199 closed pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

Posted by GitBox <gi...@apache.org>.
mf2199 closed pull request #11295:
URL: https://github.com/apache/beam/pull/11295


   


----------------------------------------------------------------
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] [beam] mf2199 commented on a change in pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

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



##########
File path: sdks/python/apache_beam/io/gcp/bigtableio.py
##########
@@ -1,151 +0,0 @@
-#
-# Licensed to the Apache Software Foundation (ASF) under one or more
-# contributor license agreements.  See the NOTICE file distributed with
-# this work for additional information regarding copyright ownership.
-# The ASF licenses this file to You under the Apache License, Version 2.0
-# (the "License"); you may not use this file except in compliance with
-# the License.  You may obtain a copy of the License at
-#
-#    http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-"""BigTable connector
-
-This module implements writing to BigTable tables.

Review comment:
       If I understand correctly, we are to restore this file in the original version while keeping the altered one in the `experimental` folder. If so, then it's done.




----------------------------------------------------------------
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] [beam] chamikaramj commented on pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

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


   Pre-commit failure seems to be related.


----------------------------------------------------------------
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] [beam] mf2199 edited a comment on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

Posted by GitBox <gi...@apache.org>.
mf2199 edited a comment on pull request #11295:
URL: https://github.com/apache/beam/pull/11295#issuecomment-647196061


   @chamikaramj @aaltay The build errors point to 'missing' arguments that were made so by design, e.g.:
   ```
   15:41:25 E       ValueError: Pipeline has validations errors: 
   15:41:25 E       Missing required option: project.
   15:41:25 E       Missing required option: region.
   15:41:25 E       Missing GCS path option: temp_location.
   15:41:25 E       Missing GCS path option: staging_location.
   ```
   [from "Run Python PreCommit" log]
   
   These are meant to be supplied via command line, as they may contain non-public information and are subject to change. 
   
   Any ideas?


----------------------------------------------------------------
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] [beam] param17 commented on pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

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


   Can we re-open this? What's the next step?
   cc: @mf2199 


-- 
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] [beam] mf2199 commented on pull request #11295: [WIP-draft, testing only - NOT READY FOR REVIEW]

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


   Run Python PreCommit


----------------------------------------------------------------
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] [beam] chamikaramj commented on a change in pull request #11295: [BEAM-3342] CBT `Read` IO Connector Wrapper

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



##########
File path: sdks/python/apache_beam/io/gcp/bigtableio.py
##########
@@ -1,151 +0,0 @@
-#
-# Licensed to the Apache Software Foundation (ASF) under one or more
-# contributor license agreements.  See the NOTICE file distributed with
-# this work for additional information regarding copyright ownership.
-# The ASF licenses this file to You under the Apache License, Version 2.0
-# (the "License"); you may not use this file except in compliance with
-# the License.  You may obtain a copy of the License at
-#
-#    http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-"""BigTable connector
-
-This module implements writing to BigTable tables.

Review comment:
       Unfortunately we forgot to mark this as experimental. So we'll have to leave the sink here and add the new source to "experimental/bigtableio.py" for backwards compatibility. When we are confident about the performance of the source we can move it her. For example, I think we need to add support for dynamic work rebalancing similar to Java BigTable source (can be in a future PR).

##########
File path: sdks/python/apache_beam/io/gcp/experimental/bigtableio_read_it_test.py
##########
@@ -0,0 +1,169 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+""" Integration test for GC Bigtable connector [read]."""
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+import argparse
+import logging
+import unittest
+
+from nose.plugins.attrib import attr
+
+import apache_beam as beam
+from apache_beam.io.gcp.experimental.bigtableio import ReadFromBigtable
+from apache_beam.metrics.metric import MetricsFilter
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import SetupOptions
+from apache_beam.runners.runner import PipelineState
+
+try:
+  from google.cloud.bigtable import Client
+except ImportError:
+  Client = None
+
+
+@unittest.skipIf(Client is None, 'GC Bigtable dependencies are not installed')
+class BigtableReadTest(unittest.TestCase):
+  """ Bigtable Read Connector Test
+
+  This tests the ReadFromBigtable connector class via reading rows from
+  a Bigtable table and comparing the `Rows Read` metrics with the row
+  count known a priori.
+  """
+  def setUp(self):
+    self.options = parse_commane_line_arguments()
+
+    logging.info('\nProject ID:  %s', self.options['project'])
+    logging.info('\nInstance ID: %s', self.options['instance'])
+    logging.info('\nTable ID:    %s', self.options['table'])
+
+    self._p_options = PipelineOptions(**self.options)
+    self._p_options.view_as(SetupOptions).save_main_session = True
+
+    logging.getLogger().setLevel(self.options['log_level'])
+
+    # [OPTIONAL] Uncomment to save logs into a file
+    # self._setup_log_file()
+
+    # [OPTIONAL] Uncomment this to allow logging the pipeline options
+    # for key, value in self.p_options.get_all_options().items():
+    #   logging.info('Pipeline option {:32s} : {}'.format(key, value))
+
+  def _setup_log_file(self):
+    if self.options['log_directory']:
+      # logging.basicConfig(
+      #   filename='{}{}.log'.format(
+      #       options['log_directory'], options['table']
+      #   ),
+      #   filemode='w',
+      #   level=logging.DEBUG
+      # )
+
+      # Forward all the logs to a file
+      fh = logging.FileHandler(
+          filename='{}test_log_{}_RUNNER={}.log'.format(
+              self.options['log_directory'],
+              self.options['table'][-15:],
+              self.options['runner']))
+      fh.setLevel(logging.DEBUG)
+      logging.getLogger().addHandler(fh)
+
+  @attr('IT')
+  def test_bigtable_read(self):
+    logging.info(
+        'Reading table "%s" of %d rows...',
+        self.options['table'],
+        self.options['row_count'] or 0)
+
+    p = beam.Pipeline(options=self._p_options)
+    _ = (
+        p | 'Read Test' >> ReadFromBigtable(
+            project_id=self.options['project'],
+            instance_id=self.options['instance'],
+            table_id=self.options['table'],
+            filter_=self.options['filter']))
+    self.result = p.run()
+    self.result.wait_until_finish()
+    assert self.result.state == PipelineState.DONE
+
+    query_result = self.result.metrics().query(
+        MetricsFilter().with_name('Rows Read'))
+
+    if query_result['counters']:
+      read_counter = query_result['counters'][0]
+      final_count = read_counter.committed
+      assert final_count == self.options['row_count']
+      logging.info(
+          '%d out of %d rows were read successfully.',
+          final_count,
+          self.options['row_count'])
+
+    logging.info('DONE!')
+
+
+def parse_commane_line_arguments():
+  parser = argparse.ArgumentParser()
+

Review comment:
       You do not need to set these up manually. Please see the way our other integration tests are setup. You can either configure a new Jenkins test suite or add integration tests to existing Python post-commit tests. Latter might be easier.




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