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/07/17 12:10:10 UTC

[GitHub] [beam] piotr-szuberski opened a new pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

piotr-szuberski opened a new pull request #12297:
URL: https://github.com/apache/beam/pull/12297


   This transform sends bytes stream instead of the whole KinesisRecord, as RowCoder is not fully functional yet.
   
   This change depends on #12234 and #12235 - please review only the commits related to the Read transform.
   
   R: @chamikaramj 
   ------------------------
   
   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 | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
   Java | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
   XLang | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
   Portable | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.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] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **increase** coverage by `0.02%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   + Coverage   40.22%   40.24%   +0.02%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    53717      +48     
   ==========================================
   + Hits        21587    21619      +32     
   - Misses      32082    32098      +16     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-704757337


   > I'm getting this error when trying `ReadDataFromKinesis`,
   > 
   > > RuntimeError: Unable to fetch remote job server jar at https://repo.maven.apache.org/maven2/org/apache/beam/beam-sdks-java-io-kinesis-expansion-service/2.24.0/beam-sdks-java-io-kinesis-expansion-service-2.24.0.jar: HTTP Error 404: Not Found
   > 
   > Seems like **beam-sdks-java-io-kinesis-expansion-service** no longer exists on Maven?
   
   @yuanhunglo ReadDataFromKinesis transform will be available in the 2.25.0 release. It was merged too late to make it to the 2.24.0. If you want to run it using 2.25 code then you need to build the expansion service using
   ```
   ./gradlew :sdks:java:io:kinesis:expansion-service:shadowJar
   ```
   Then ReadDataFromKinesis should be able to detect the expansion service jar in its default path


----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-706002739


   Sorry, I forgot there is an additional setup needed.
   
   You also need to build the jar of the runner you are using - example for Flink (you don't need it for DirectRunner)
   ```
   ./gradlew :runners:flink:1.10:job-server:shadowJar
   ```
   Current python container:
   ```
   ./gradlew :sdks:python:container:py37:docker
   ```
   Building java sdk container should not be needed - pulling the latest (2.24) from docker hub should be sufficient. But to be on the safe side:
   ```
   ./gradlew :sdks:java:container:docker
   docker tag apache/beam_java8_sdk:2.26.0.dev apache/beam_java8_sdk:2.24.0.dev 
   ```
   
   It will definitely be the easiest to wait for 2.25 but on the other hand it's a long 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] piotr-szuberski removed a comment on pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski removed a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-665178830






----------------------------------------------------------------
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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r469051994



##########
File path: sdks/python/apache_beam/io/kinesis.py
##########
@@ -0,0 +1,317 @@
+#
+# 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.
+#
+
+"""PTransforms for supporting Kinesis streaming in Python pipelines.
+
+  These transforms are currently supported by Beam Flink and Spark portable
+  runners.
+
+  **Setup**
+
+  Transforms provided in this module are cross-language transforms
+  implemented in the Beam Java SDK. During the pipeline construction, Python SDK
+  will connect to a Java expansion service to expand these transforms.
+  To facilitate this, a small amount of setup is needed before using these
+  transforms in a Beam Python pipeline.
+
+  There are several ways to setup cross-language Kinesis transforms.
+
+  * Option 1: use the default expansion service
+  * Option 2: specify a custom expansion service
+
+  See below for details regarding each of these options.
+
+  *Option 1: Use the default expansion service*
+
+  This is the recommended and easiest setup option for using Python Kinesis
+  transforms. This option is only available for Beam 2.24.0 and later.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Install Java runtime in the computer from where the pipeline is constructed
+    and make sure that 'java' command is available.
+
+  In this option, Python SDK will either download (for released Beam version) or
+  build (when running from a Beam Git clone) a expansion service jar and use
+  that to expand transforms. Currently Kinesis transforms use the
+  'beam-sdks-java-io-kinesis-expansion-service' jar for this purpose.
+
+  *Option 2: specify a custom expansion service*
+
+  In this option, you startup your own expansion service and provide that as
+  a parameter when using the transforms provided in this module.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Startup your own expansion service.
+  * Update your pipeline to provide the expansion service address when
+    initiating Kinesis transforms provided in this module.
+
+  Flink Users can use the built-in Expansion Service of the Flink Runner's
+  Job Server. If you start Flink's Job Server, the expansion service will be
+  started on port 8097. For a different address, please set the
+  expansion_service parameter.
+
+  **More information**
+
+  For more information regarding cross-language transforms see:
+  - https://beam.apache.org/roadmap/portability/
+
+  For more information specific to Flink runner see:
+  - https://beam.apache.org/documentation/runners/flink/
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import time
+from typing import List
+from typing import NamedTuple
+from typing import Optional
+from typing import Tuple
+
+from past.builtins import unicode
+
+from apache_beam import BeamJarExpansionService
+from apache_beam import ExternalTransform
+from apache_beam import NamedTupleBasedPayloadBuilder
+
+__all__ = [
+    'WriteToKinesis',
+    'ReadDataFromKinesis',
+    'InitialPositionInStream',
+    'WatermarkPolicy',
+]
+
+
+def default_io_expansion_service():
+  return BeamJarExpansionService(
+      'sdks:java:io:kinesis:expansion-service:shadowJar')
+
+
+WriteToKinesisSchema = NamedTuple(
+    'WriteToKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('partition_key', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('producer_properties', Optional[List[Tuple[unicode, unicode]]]),
+    ],
+)
+
+
+class WriteToKinesis(ExternalTransform):
+  """
+    An external PTransform which writes byte array stream to Amazon Kinesis.
+
+    Experimental; no backwards compatibility guarantees.
+  """
+  URN = 'beam:external:java:kinesis:write:v1'
+
+  def __init__(
+      self,
+      stream_name,
+      aws_access_key,
+      aws_secret_key,
+      region,
+      partition_key,
+      service_endpoint=None,
+      verify_certificate=None,
+      producer_properties=None,
+      expansion_service=None,
+  ):
+    """
+    Initializes a write operation to Kinesis.
+
+    :param stream_name: Kinesis stream name.
+    :param aws_access_key: Kinesis access key.
+    :param aws_secret_key: Kinesis access key secret.
+    :param region: AWS region. Example: 'us-east-1'.
+    :param service_endpoint: Kinesis service endpoint
+    :param verify_certificate: Enable or disable certificate verification.
+        Never set to False on production. True by default.
+    :param partition_key: Specify default partition key.
+    :param producer_properties: Specify the configuration properties for Kinesis
+        Producer Library (KPL) as List[KV[string, string]].
+        Example: [('CollectionMaxCount', '1000'), ('ConnectTimeout', '10000')]
+    :param expansion_service: The address (host:port) of the ExpansionService.
+    """
+    super(WriteToKinesis, self).__init__(
+        self.URN,
+        NamedTupleBasedPayloadBuilder(
+            WriteToKinesisSchema(
+                stream_name=stream_name,
+                aws_access_key=aws_access_key,
+                aws_secret_key=aws_secret_key,
+                region=region,
+                partition_key=partition_key,
+                service_endpoint=service_endpoint,
+                verify_certificate=verify_certificate,
+                producer_properties=producer_properties,
+            )),
+        expansion_service or default_io_expansion_service(),
+    )
+
+
+ReadFromKinesisSchema = NamedTuple(
+    'ReadFromKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('max_num_records', Optional[int]),
+        ('max_read_time', Optional[int]),
+        ('initial_position_in_stream', Optional[unicode]),
+        ('initial_timestamp_in_stream', Optional[int]),
+        ('request_records_limit', Optional[int]),
+        ('up_to_date_threshold', Optional[int]),
+        ('max_capacity_per_shard', Optional[int]),
+        ('watermark_policy', Optional[unicode]),
+        ('watermark_idle_duration_threshold', Optional[int]),
+        ('rate_limit', Optional[int]),
+    ],
+)
+
+
+class InitialPositionInStream:
+  LATEST = 'LATEST'
+  TRIM_HORIZON = 'TRIM_HORIZON'
+  AT_TIMESTAMP = 'AT_TIMESTAMP'
+
+
+class WatermarkPolicy:
+  PROCESSING_TYPE = 'PROCESSING_TYPE'
+  ARRIVAL_TIME = 'ARRIVAL_TIME'
+
+
+class ReadDataFromKinesis(ExternalTransform):
+  """
+    An external PTransform which reads byte array stream from Amazon Kinesis.
+
+    Experimental; no backwards compatibility guarantees.
+  """
+  URN = 'beam:external:java:kinesis:read_data:v1'
+
+  def __init__(
+      self,
+      stream_name,
+      aws_access_key,
+      aws_secret_key,
+      region,
+      service_endpoint=None,
+      verify_certificate=None,
+      max_num_records=None,
+      max_read_time=None,
+      initial_position_in_stream=None,
+      initial_timestamp_in_stream=None,
+      request_records_limit=None,
+      up_to_date_threshold=None,
+      max_capacity_per_shard=None,
+      watermark_policy=None,
+      watermark_idle_duration_threshold=None,
+      rate_limit=None,
+      expansion_service=None,
+  ):
+    """
+    Initializes a read operation from Kinesis.
+
+    :param stream_name: Kinesis stream name.
+    :param aws_access_key: Kinesis access key.
+    :param aws_secret_key: Kinesis access key secret.
+    :param region: AWS region. Example: 'us-east-1'.
+    :param service_endpoint: Kinesis service endpoint
+    :param verify_certificate: Enable or disable certificate verification.
+        Never set to False on production. True by default.
+    :param max_num_records: Specifies to read at most a given number of records.
+        Must be greater than 0.
+    :param max_read_time: Specifies to read records during x seconds.
+    :param initial_timestamp_in_stream: Specify reading beginning at the given
+        timestamp in seconds. Must be in the past.
+    :param initial_position_in_stream: Specify reading from some initial
+        position in stream. Possible values:
+        LATEST - Start after the most recent data record (fetch new data).
+        TRIM_HORIZON - Start from the oldest available data record.
+        AT_TIMESTAMP - Start from the record at or after the specified
+        server-side timestamp.
+    :param request_records_limit: Specifies the maximum number of records in
+        GetRecordsResult returned by GetRecords call which is limited by 10K
+        records. If should be adjusted according to average size of data record
+        to prevent shard overloading. More at:
+        docs.aws.amazon.com/kinesis/latest/APIReference/API_GetRecords.html
+    :param up_to_date_threshold: Specifies how late in seconds records consumed
+        by this source can be to still be considered on time. Defaults to zero.
+    :param max_capacity_per_shard: Specifies the maximum number of messages per
+        one shard. Defaults to 10'000.
+    :param watermark_policy: Specifies the watermark policy. Possible values:
+        PROCESSING_TYPE, ARRIVAL_TIME. Defaults to ARRIVAL_TIME.
+    :param watermark_idle_duration_threshold: Use only when watermark policy is
+        ARRIVAL_TIME. Denotes the duration for which the watermark can be idle.
+        Passed in seconds.
+    :param rate_limit: Sets fixed rate policy for given seconds value. By
+        default there is no rate limit.
+    :param expansion_service: The address (host:port) of the ExpansionService.
+    """
+    if watermark_policy:
+      assert watermark_policy == WatermarkPolicy.ARRIVAL_TIME or\
+             watermark_policy == WatermarkPolicy.PROCESSING_TYPE
+
+    if initial_position_in_stream:
+      i = initial_position_in_stream
+      assert i == InitialPositionInStream.AT_TIMESTAMP or \
+             i == InitialPositionInStream.LATEST or \
+             i == InitialPositionInStream.TRIM_HORIZON
+
+    if request_records_limit:
+      assert 0 < request_records_limit <= 10000
+
+    if initial_timestamp_in_stream:
+      assert initial_timestamp_in_stream < time.time()

Review comment:
       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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r468394074



##########
File path: sdks/python/apache_beam/io/external/xlang_kinesisio_it_test.py
##########
@@ -0,0 +1,304 @@
+#
+# 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 Python cross-language pipelines for Java KinesisIO.
+
+If you want to run the tests on localstack then run it just with pipeline
+options.
+
+To test it on a real AWS account you need to pass some additional params, e.g.:
+python setup.py nosetests \
+--tests=apache_beam.io.external.xlang_kinesisio_it_test \
+--test-pipeline-options="
+  --use_real_aws
+  --aws_kinesis_stream=<STREAM_NAME>
+  --aws_access_key=<AWS_ACCESS_KEY>
+  --aws_secret_key=<AWS_SECRET_KEY>
+  --aws_region=<AWS_REGION>
+  --runner=FlinkRunner"
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import argparse
+import logging
+import time
+import unittest
+import uuid
+
+import apache_beam as beam
+from apache_beam.io.kinesis import InitialPositionInStream
+from apache_beam.io.kinesis import ReadDataFromKinesis
+from apache_beam.io.kinesis import WriteToKinesis
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import StandardOptions
+from apache_beam.testing.test_pipeline import TestPipeline
+from apache_beam.testing.util import assert_that
+from apache_beam.testing.util import equal_to
+
+# pylint: disable=wrong-import-order, wrong-import-position, ungrouped-imports
+try:
+  import boto3
+except ImportError:
+  boto3 = None
+
+try:
+  from testcontainers.core.container import DockerContainer
+except ImportError:
+  DockerContainer = None
+# pylint: enable=wrong-import-order, wrong-import-position, ungrouped-imports
+
+LOCALSTACK_VERSION = '0.11.3'
+NUM_RECORDS = 10
+NOW = time.time()
+RECORD = b'record' + str(uuid.uuid4()).encode()
+
+
+@unittest.skipIf(DockerContainer is None, 'testcontainers is not installed.')
+@unittest.skipIf(boto3 is None, 'boto3 is not installed.')
+@unittest.skipIf(
+    TestPipeline().get_pipeline_options().view_as(StandardOptions).runner is
+    None,
+    'Do not run this test on precommit suites.',
+)
+class CrossLanguageKinesisIOTest(unittest.TestCase):
+  def test_kinesis_io(self):
+    self.run_kinesis_write()
+    # TODO: remove once BEAM-10664 is resolved
+    if not self.use_localstack:
+      self.run_kinesis_read_data()
+
+  def run_kinesis_write(self):
+    with TestPipeline(options=PipelineOptions(self.pipeline_args)) as p:
+      p.not_use_test_runner_api = True
+      _ = (
+          p
+          | 'Impulse' >> beam.Impulse()
+          | 'Generate' >> beam.FlatMap(lambda x: range(NUM_RECORDS))  # pylint: disable=range-builtin-not-iterating
+          | 'Map to bytes' >>
+          beam.Map(lambda x: RECORD + str(x).encode()).with_output_types(bytes)
+          | 'WriteToKinesis' >> WriteToKinesis(
+              stream_name=self.aws_kinesis_stream,
+              aws_access_key=self.aws_access_key,
+              aws_secret_key=self.aws_secret_key,
+              region=self.aws_region,
+              service_endpoint=self.aws_service_endpoint,
+              verify_certificate=(not self.use_localstack),
+              partition_key='1',
+          ))
+
+    # TODO: Remove once BEAM-10664 is resolved
+    if self.use_localstack:
+      records = self.kinesis_helper.read_from_stream(self.aws_kinesis_stream)
+      self.assertEqual(
+          sorted(records),
+          sorted([RECORD + str(i).encode() for i in range(NUM_RECORDS)]))

Review comment:
       I'll not write a separate read test for now because this can't be tested anyway and could be confusing later if there is some bug. I think that it'd be better to just leave test_kinesis_io_roundtrip and remove test_write once the problem is solved - it would reduce the amount of boto3 used in the test.




----------------------------------------------------------------
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] piotr-szuberski removed a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski removed a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666352407


   Run Python 3.8 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] piotr-szuberski removed a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski removed a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666408845


   Run Python 3.8 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] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **decrease** coverage by `0.12%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   - Coverage   40.22%   40.10%   -0.13%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    54025     +356     
   ==========================================
   + Hits        21587    21665      +78     
   - Misses      32082    32360     +278     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   | [.../runners/portability/fn\_api\_runner/translations.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3RyYW5zbGF0aW9ucy5weQ==) | `14.00% <0.00%> (+0.22%)` | :arrow_up: |
   | [...beam/runners/interactive/background\_caching\_job.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9iYWNrZ3JvdW5kX2NhY2hpbmdfam9iLnB5) | `25.92% <0.00%> (+0.68%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666363032


   @TheNeuralBit <del>Forget what I just said. I've just found that the expansion-service has an indirect dependency on harness and I managed to set test properties in the gradle file. Now both read and write seem to work locally, I must review why I still have certification problem on jenkins. </del>
   I don't know why python linter failed on an unrelated file: spark_uber_jar_job_server_test
   
   Edit: I'm an idiot. I was using java container with modified harness with system properties all this time.. So unfortunately the case with read test is still not solved - it would require to have a custom java container for postcommit tests which is doable but I doubt it is worth 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] [beam] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-665688027


   @TheNeuralBit I did my best to make read test work but without success. I tried what people suggested here https://github.com/localstack/localstack/issues/507 and https://stackoverflow.com/a/56867870/7946496 but nothing worked.
   
   When I use http in service_endpoint, which works for boto3 python sdk I get error:
   `com.amazonaws.services.kinesis.model.AmazonKinesisException: null (Service: AmazonKinesis; Status Code: 500; Error Code: null; Request ID: null)`
   
   When I use https in the transform argument service_endpoint I get
   `sun.security.validator.ValidatorException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target`
   I tried to run jars with these flags: `-Dcom.sun.net.ssl.checkRevocation=false`, `-Dtrust_all_cert=true` `-Dcom.amazonaws.sdk.disableCertChecking`
   
   Setting these values in the main method of expansion service also had no effect.
   ```
       System.setProperty(SDKGlobalConfiguration.DISABLE_CERT_CHECKING_SYSTEM_PROPERTY, "true");
       System.setProperty(SDKGlobalConfiguration.AWS_CBOR_DISABLE_SYSTEM_PROPERTY, "true");
       System.setProperty("com.sun.net.ssl.checkRevocation", "true");
   ```
   
   So I'm a bit stuck. How about skipping the read test and creating a Jira issue to make it work? I think that it's possible to run this test with localstack but I'm missing something. I've spent a lot of time on it already and I doubt I'll come up with something.


----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666352407


   Run Python 3.8 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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666361077


   Run Python 3.8 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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-667100096


   Run Python 3.8 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] piotr-szuberski removed a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski removed a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-667100096






----------------------------------------------------------------
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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r474416306



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -295,14 +300,16 @@
   private static final int DEFAULT_NUM_RETRIES = 6;
 
   /** Returns a new {@link Read} transform for reading from Kinesis. */
-  public static Read read() {
-    return new AutoValue_KinesisIO_Read.Builder()
-        .setMaxNumRecords(Long.MAX_VALUE)
-        .setUpToDateThreshold(Duration.ZERO)
-        .setWatermarkPolicyFactory(WatermarkPolicyFactory.withArrivalTimePolicy())
-        .setRateLimitPolicyFactory(RateLimitPolicyFactory.withoutLimiter())
-        .setMaxCapacityPerShard(ShardReadersPool.DEFAULT_CAPACITY_PER_SHARD)
-        .build();
+  public static Read<KinesisRecord> read() {

Review comment:
       The worst thing I can imagine is that a user will get a raw type warning if he used it like `KinesisIO.Read read = KinesisIO.read();`. Some users could have some checkers that would turn it into an error though. I don't know how much common such tools are in the Java development in projects using Beam.
   `PCollection<KinesisRecord> = pipeline.apply(KinesisIO.read()...));` doesn't change so I think it's ok.




----------------------------------------------------------------
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] piotr-szuberski removed a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski removed a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666458687


   Run Python 3.8 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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-671271227


   @TheNeuralBit Ping


----------------------------------------------------------------
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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r474416306



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -295,14 +300,16 @@
   private static final int DEFAULT_NUM_RETRIES = 6;
 
   /** Returns a new {@link Read} transform for reading from Kinesis. */
-  public static Read read() {
-    return new AutoValue_KinesisIO_Read.Builder()
-        .setMaxNumRecords(Long.MAX_VALUE)
-        .setUpToDateThreshold(Duration.ZERO)
-        .setWatermarkPolicyFactory(WatermarkPolicyFactory.withArrivalTimePolicy())
-        .setRateLimitPolicyFactory(RateLimitPolicyFactory.withoutLimiter())
-        .setMaxCapacityPerShard(ShardReadersPool.DEFAULT_CAPACITY_PER_SHARD)
-        .build();
+  public static Read<KinesisRecord> read() {

Review comment:
       The worst thing I can imagine is that a user will get a raw type warning if he used it like `KinesisIO.Read read = KinesisIO.read();`. Some users could have some checkers that would turn it into an error though. I don't know how much common such tools are in the Java development.




----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-671293518


   Run Python 3.8 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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666279110


   @TheNeuralBit Sorry for lots of messages but every day brings something new.
   
   I've managed to modify existing KinesisIOIT to work with localstack (I'll push it in a separate PR).
   
   I've also managed to run cross-language read test. But it required putting this code anywhere in harness code, which is definitely not any solution:
   ```
   System.setProperty("com.amazonaws.sdk.disableCertChecking", "true");
   System.setProperty("com.amazonaws.sdk.disableCbor", "true");
   ```
   
   I have no idea how to dynamically inject these except for creating custom beam java sdk docker image with modified harness code especially for this test. I don't think it's an acceptable solution though. Do you possibly have a better idea?
   
   If this is the only thing we could do I'd just skip read test and just execute only write instead in the 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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-706002739


   Sorry, I forgot there is an additional setup needed.
   
   You also need to build the jar of the runner you are using - example for Flink (you don't need it for DirectRunner)
   ```
   ./gradlew :runners:flink:1.10:job-server:shadowJar
   ```
   Current python container:
   ```
   ./gradlew :sdks:python:container:py37:docker
   ```
   Building java sdk container should not be needed - pulling the latest (2.24) from docker hub should be sufficient. But to be on the safe side:
   ```
   ./gradlew :sdks:java:container:docker
   docker tag apache/beam_java8_sdk:2.26.0.dev apache/beam_java8_sdk:2.24.0.dev 
   ```
   
   It will definitely be the easiest to wait for 2.25 but on the other hand it's a long 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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-704757337


   > I'm getting this error when trying `ReadDataFromKinesis`,
   > 
   > > RuntimeError: Unable to fetch remote job server jar at https://repo.maven.apache.org/maven2/org/apache/beam/beam-sdks-java-io-kinesis-expansion-service/2.24.0/beam-sdks-java-io-kinesis-expansion-service-2.24.0.jar: HTTP Error 404: Not Found
   > 
   > Seems like **beam-sdks-java-io-kinesis-expansion-service** no longer exists on Maven?
   
   @yuanhunglo ReadDataFromKinesis transform will be available in the 2.25.0 release. It was merged too late to make it to the 2.24.0. If you want to run it using 2.25 code then you need to build the expansion service using
   ```
   ./gradlew :sdks:java:io:kinesis:expansion-service:shadowJar
   ```
   Then ReadDataFromKinesis should be able to detect the expansion service jar


----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **increase** coverage by `0.02%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   + Coverage   40.22%   40.24%   +0.02%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    53717      +48     
   ==========================================
   + Hits        21587    21619      +32     
   - Misses      32082    32098      +16     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r459276795



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -561,6 +569,41 @@ public Read withMaxCapacityPerShard(Integer maxCapacity) {
     }
   }
 
+  /**
+   * A {@link PTransform} to read from Kinesis stream. Similar to {@link KinesisIO.Read}, but
+   * removes Kinesis metatdata and returns a {@link PCollection} of {@link byte[]}. See {@link
+   * KinesisIO} for more information on usage and configuration of reader.
+   */
+  public static class TypedWithoutMetadata extends PTransform<PBegin, PCollection<byte[]>> {

Review comment:
       I changed the URN to read_data, I think it's self-explanatory for a kinesis user.




----------------------------------------------------------------
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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r459261349



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -536,6 +537,13 @@ public Read withMaxCapacityPerShard(Integer maxCapacity) {
       return toBuilder().setMaxCapacityPerShard(maxCapacity).build();
     }
 
+    /**
+     * Returns a {@link PTransform} for PCollection of {@link byte[]}, dropping Kinesis metatdata.
+     */
+    public PTransform<PBegin, PCollection<byte[]>> withoutMetadata() {
+      return new TypedWithoutMetadata(this);
+    }

Review comment:
       Thanks for pointing it, I followed the KafkaIO convention and didn't think whether it's a good design or not. The change is trivial, just use KinesisIO.readWithoutMetadata() and pass Read there instead of creating it in withoutMetadata(this).




----------------------------------------------------------------
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] aromanenko-dev commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
aromanenko-dev commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-685454911


   Thanks for make it finally merged. 
   Quick note: this change is quite significant, please, update `CHANGES.md` on this (if not yet).


----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **increase** coverage by `0.02%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   + Coverage   40.22%   40.24%   +0.02%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    53717      +48     
   ==========================================
   + Hits        21587    21619      +32     
   - Misses      32082    32098      +16     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666363032


   @TheNeuralBit Forget what I just said. I've just found that the expansion-service has an indirect dependency on harness and I managed to set test properties in the gradle file. Now both read and write seem to work locally, I must review why I still have certification problem on jenkins. 
   I don't know why python linter failed on an unrelated file: spark_uber_jar_job_server_test


----------------------------------------------------------------
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] yuanhunglo commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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


   I'm getting this error when trying `ReadDataFromKinesis`, 
   
   > RuntimeError: Unable to fetch remote job server jar at https://repo.maven.apache.org/maven2/org/apache/beam/beam-sdks-java-io-kinesis-expansion-service/2.24.0/beam-sdks-java-io-kinesis-expansion-service-2.24.0.jar: HTTP Error 404: Not Found
   
   Seems like **beam-sdks-java-io-kinesis-expansion-service** no longer exists on Maven? 


----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-672974579


   Run Python 3.8 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] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **increase** coverage by `0.02%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   + Coverage   40.22%   40.24%   +0.02%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    53717      +48     
   ==========================================
   + Hits        21587    21619      +32     
   - Misses      32082    32098      +16     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-675901994


   @chamikaramj ping


----------------------------------------------------------------
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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-706002739


   Sorry, I forgot there is some additional setup needed to run it locally.
   
   You also need to build the jar of the runner you are using - example for Flink (you don't need it for DirectRunner)
   ```
   ./gradlew :runners:flink:1.10:job-server:shadowJar
   ```
   Current python container:
   ```
   ./gradlew :sdks:python:container:py37:docker
   ```
   Building java sdk container should not be needed - pulling the latest (2.24) from docker hub should be sufficient. But to be on the safe side:
   ```
   ./gradlew :sdks:java:container:docker
   docker tag apache/beam_java8_sdk:2.26.0.dev apache/beam_java8_sdk:2.24.0.dev 
   ```
   
   It will definitely be the easiest to wait for 2.25 but on the other hand it's a long 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] chamikaramj commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -295,14 +300,16 @@
   private static final int DEFAULT_NUM_RETRIES = 6;
 
   /** Returns a new {@link Read} transform for reading from Kinesis. */
-  public static Read read() {
-    return new AutoValue_KinesisIO_Read.Builder()
-        .setMaxNumRecords(Long.MAX_VALUE)
-        .setUpToDateThreshold(Duration.ZERO)
-        .setWatermarkPolicyFactory(WatermarkPolicyFactory.withArrivalTimePolicy())
-        .setRateLimitPolicyFactory(RateLimitPolicyFactory.withoutLimiter())
-        .setMaxCapacityPerShard(ShardReadersPool.DEFAULT_CAPACITY_PER_SHARD)
-        .build();
+  public static Read<KinesisRecord> read() {

Review comment:
       Prob. this is OK as long as the change is backwards compatible for user pipelines.




----------------------------------------------------------------
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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r459261482



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -536,6 +537,13 @@ public Read withMaxCapacityPerShard(Integer maxCapacity) {
       return toBuilder().setMaxCapacityPerShard(maxCapacity).build();
     }
 
+    /**
+     * Returns a {@link PTransform} for PCollection of {@link byte[]}, dropping Kinesis metatdata.
+     */
+    public PTransform<PBegin, PCollection<byte[]>> withoutMetadata() {
+      return new TypedWithoutMetadata(this);
+    }

Review comment:
       I'll leave it in KinesisIO.




----------------------------------------------------------------
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] TheNeuralBit commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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






----------------------------------------------------------------
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] piotr-szuberski removed a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski removed a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666382582


   Run Python 3.8 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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r468391828



##########
File path: sdks/python/apache_beam/io/kinesis.py
##########
@@ -0,0 +1,317 @@
+#
+# 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.
+#
+
+"""PTransforms for supporting Kinesis streaming in Python pipelines.
+
+  These transforms are currently supported by Beam Flink and Spark portable
+  runners.
+
+  **Setup**
+
+  Transforms provided in this module are cross-language transforms
+  implemented in the Beam Java SDK. During the pipeline construction, Python SDK
+  will connect to a Java expansion service to expand these transforms.
+  To facilitate this, a small amount of setup is needed before using these
+  transforms in a Beam Python pipeline.
+
+  There are several ways to setup cross-language Kinesis transforms.
+
+  * Option 1: use the default expansion service
+  * Option 2: specify a custom expansion service
+
+  See below for details regarding each of these options.
+
+  *Option 1: Use the default expansion service*
+
+  This is the recommended and easiest setup option for using Python Kinesis
+  transforms. This option is only available for Beam 2.24.0 and later.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Install Java runtime in the computer from where the pipeline is constructed
+    and make sure that 'java' command is available.
+
+  In this option, Python SDK will either download (for released Beam version) or
+  build (when running from a Beam Git clone) a expansion service jar and use
+  that to expand transforms. Currently Kinesis transforms use the
+  'beam-sdks-java-io-kinesis-expansion-service' jar for this purpose.
+
+  *Option 2: specify a custom expansion service*
+
+  In this option, you startup your own expansion service and provide that as
+  a parameter when using the transforms provided in this module.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Startup your own expansion service.
+  * Update your pipeline to provide the expansion service address when
+    initiating Kinesis transforms provided in this module.
+
+  Flink Users can use the built-in Expansion Service of the Flink Runner's
+  Job Server. If you start Flink's Job Server, the expansion service will be
+  started on port 8097. For a different address, please set the
+  expansion_service parameter.
+
+  **More information**
+
+  For more information regarding cross-language transforms see:
+  - https://beam.apache.org/roadmap/portability/
+
+  For more information specific to Flink runner see:
+  - https://beam.apache.org/documentation/runners/flink/
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import time
+from typing import List
+from typing import NamedTuple
+from typing import Optional
+from typing import Tuple
+
+from past.builtins import unicode
+
+from apache_beam import BeamJarExpansionService
+from apache_beam import ExternalTransform
+from apache_beam import NamedTupleBasedPayloadBuilder
+
+__all__ = [
+    'WriteToKinesis',
+    'ReadDataFromKinesis',
+    'InitialPositionInStream',
+    'WatermarkPolicy',
+]
+
+
+def default_io_expansion_service():
+  return BeamJarExpansionService(
+      'sdks:java:io:kinesis:expansion-service:shadowJar')
+
+
+WriteToKinesisSchema = NamedTuple(
+    'WriteToKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('partition_key', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('producer_properties', Optional[List[Tuple[unicode, unicode]]]),

Review comment:
       Great, Mapping will at least be there! I'll follow 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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-672727263


   Run Python 3.8 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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666279110


   @TheNeuralBit Sorry for lots of messages but every day brings something new.
   
   I've managed to modify existing KinesisIOIT to work with localstack (I'll push it in a separate PR).
   
   I've also managed to run cross-language read test. But it required putting this code anywhere in harness code, which is definitely not any solution because it affects whole beam:
   ```
   System.setProperty("com.amazonaws.sdk.disableCertChecking", "true");
   System.setProperty("com.amazonaws.sdk.disableCbor", "true");
   ```
   
   I have no idea how to dynamically inject these except for creating custom beam java sdk docker image with modified harness code especially for this (read) test. I don't think it's an acceptable solution though. Do you possibly have a better idea?
   
   If this is the only thing we could do I'd just skip read test and just execute only write instead in the 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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r469072984



##########
File path: sdks/python/apache_beam/io/kinesis.py
##########
@@ -0,0 +1,317 @@
+#
+# 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.
+#
+
+"""PTransforms for supporting Kinesis streaming in Python pipelines.
+
+  These transforms are currently supported by Beam Flink and Spark portable
+  runners.
+
+  **Setup**
+
+  Transforms provided in this module are cross-language transforms
+  implemented in the Beam Java SDK. During the pipeline construction, Python SDK
+  will connect to a Java expansion service to expand these transforms.
+  To facilitate this, a small amount of setup is needed before using these
+  transforms in a Beam Python pipeline.
+
+  There are several ways to setup cross-language Kinesis transforms.
+
+  * Option 1: use the default expansion service
+  * Option 2: specify a custom expansion service
+
+  See below for details regarding each of these options.
+
+  *Option 1: Use the default expansion service*
+
+  This is the recommended and easiest setup option for using Python Kinesis
+  transforms. This option is only available for Beam 2.24.0 and later.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Install Java runtime in the computer from where the pipeline is constructed
+    and make sure that 'java' command is available.
+
+  In this option, Python SDK will either download (for released Beam version) or
+  build (when running from a Beam Git clone) a expansion service jar and use
+  that to expand transforms. Currently Kinesis transforms use the
+  'beam-sdks-java-io-kinesis-expansion-service' jar for this purpose.
+
+  *Option 2: specify a custom expansion service*
+
+  In this option, you startup your own expansion service and provide that as
+  a parameter when using the transforms provided in this module.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Startup your own expansion service.
+  * Update your pipeline to provide the expansion service address when
+    initiating Kinesis transforms provided in this module.
+
+  Flink Users can use the built-in Expansion Service of the Flink Runner's
+  Job Server. If you start Flink's Job Server, the expansion service will be
+  started on port 8097. For a different address, please set the
+  expansion_service parameter.
+
+  **More information**
+
+  For more information regarding cross-language transforms see:
+  - https://beam.apache.org/roadmap/portability/
+
+  For more information specific to Flink runner see:
+  - https://beam.apache.org/documentation/runners/flink/
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import time
+from typing import List
+from typing import NamedTuple
+from typing import Optional
+from typing import Tuple
+
+from past.builtins import unicode
+
+from apache_beam import BeamJarExpansionService
+from apache_beam import ExternalTransform
+from apache_beam import NamedTupleBasedPayloadBuilder
+
+__all__ = [
+    'WriteToKinesis',
+    'ReadDataFromKinesis',
+    'InitialPositionInStream',
+    'WatermarkPolicy',
+]
+
+
+def default_io_expansion_service():
+  return BeamJarExpansionService(
+      'sdks:java:io:kinesis:expansion-service:shadowJar')
+
+
+WriteToKinesisSchema = NamedTuple(
+    'WriteToKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('partition_key', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('producer_properties', Optional[List[Tuple[unicode, unicode]]]),

Review comment:
       Done. I also added some config params in the test to verify that Mapping works.




----------------------------------------------------------------
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] TheNeuralBit commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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



##########
File path: sdks/python/apache_beam/io/kinesis.py
##########
@@ -0,0 +1,317 @@
+#
+# 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.
+#
+
+"""PTransforms for supporting Kinesis streaming in Python pipelines.
+
+  These transforms are currently supported by Beam Flink and Spark portable
+  runners.
+
+  **Setup**
+
+  Transforms provided in this module are cross-language transforms
+  implemented in the Beam Java SDK. During the pipeline construction, Python SDK
+  will connect to a Java expansion service to expand these transforms.
+  To facilitate this, a small amount of setup is needed before using these
+  transforms in a Beam Python pipeline.
+
+  There are several ways to setup cross-language Kinesis transforms.
+
+  * Option 1: use the default expansion service
+  * Option 2: specify a custom expansion service
+
+  See below for details regarding each of these options.
+
+  *Option 1: Use the default expansion service*
+
+  This is the recommended and easiest setup option for using Python Kinesis
+  transforms. This option is only available for Beam 2.24.0 and later.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Install Java runtime in the computer from where the pipeline is constructed
+    and make sure that 'java' command is available.
+
+  In this option, Python SDK will either download (for released Beam version) or
+  build (when running from a Beam Git clone) a expansion service jar and use
+  that to expand transforms. Currently Kinesis transforms use the
+  'beam-sdks-java-io-kinesis-expansion-service' jar for this purpose.
+
+  *Option 2: specify a custom expansion service*
+
+  In this option, you startup your own expansion service and provide that as
+  a parameter when using the transforms provided in this module.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Startup your own expansion service.
+  * Update your pipeline to provide the expansion service address when
+    initiating Kinesis transforms provided in this module.
+
+  Flink Users can use the built-in Expansion Service of the Flink Runner's
+  Job Server. If you start Flink's Job Server, the expansion service will be
+  started on port 8097. For a different address, please set the
+  expansion_service parameter.
+
+  **More information**
+
+  For more information regarding cross-language transforms see:
+  - https://beam.apache.org/roadmap/portability/
+
+  For more information specific to Flink runner see:
+  - https://beam.apache.org/documentation/runners/flink/
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import time
+from typing import List
+from typing import NamedTuple
+from typing import Optional
+from typing import Tuple
+
+from past.builtins import unicode
+
+from apache_beam import BeamJarExpansionService
+from apache_beam import ExternalTransform
+from apache_beam import NamedTupleBasedPayloadBuilder
+
+__all__ = [
+    'WriteToKinesis',
+    'ReadDataFromKinesis',
+    'InitialPositionInStream',
+    'WatermarkPolicy',
+]
+
+
+def default_io_expansion_service():
+  return BeamJarExpansionService(
+      'sdks:java:io:kinesis:expansion-service:shadowJar')
+
+
+WriteToKinesisSchema = NamedTuple(
+    'WriteToKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('partition_key', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('producer_properties', Optional[List[Tuple[unicode, unicode]]]),

Review comment:
       #12481 is merged now so this should change to mapping (as well as producer_properties, and the corresponding parameters in Java)

##########
File path: sdks/python/apache_beam/io/kinesis.py
##########
@@ -0,0 +1,317 @@
+#
+# 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.
+#
+
+"""PTransforms for supporting Kinesis streaming in Python pipelines.
+
+  These transforms are currently supported by Beam Flink and Spark portable
+  runners.
+
+  **Setup**
+
+  Transforms provided in this module are cross-language transforms
+  implemented in the Beam Java SDK. During the pipeline construction, Python SDK
+  will connect to a Java expansion service to expand these transforms.
+  To facilitate this, a small amount of setup is needed before using these
+  transforms in a Beam Python pipeline.
+
+  There are several ways to setup cross-language Kinesis transforms.
+
+  * Option 1: use the default expansion service
+  * Option 2: specify a custom expansion service
+
+  See below for details regarding each of these options.
+
+  *Option 1: Use the default expansion service*
+
+  This is the recommended and easiest setup option for using Python Kinesis
+  transforms. This option is only available for Beam 2.24.0 and later.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Install Java runtime in the computer from where the pipeline is constructed
+    and make sure that 'java' command is available.
+
+  In this option, Python SDK will either download (for released Beam version) or
+  build (when running from a Beam Git clone) a expansion service jar and use
+  that to expand transforms. Currently Kinesis transforms use the
+  'beam-sdks-java-io-kinesis-expansion-service' jar for this purpose.
+
+  *Option 2: specify a custom expansion service*
+
+  In this option, you startup your own expansion service and provide that as
+  a parameter when using the transforms provided in this module.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Startup your own expansion service.
+  * Update your pipeline to provide the expansion service address when
+    initiating Kinesis transforms provided in this module.
+
+  Flink Users can use the built-in Expansion Service of the Flink Runner's
+  Job Server. If you start Flink's Job Server, the expansion service will be
+  started on port 8097. For a different address, please set the
+  expansion_service parameter.
+
+  **More information**
+
+  For more information regarding cross-language transforms see:
+  - https://beam.apache.org/roadmap/portability/
+
+  For more information specific to Flink runner see:
+  - https://beam.apache.org/documentation/runners/flink/
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import time
+from typing import List
+from typing import NamedTuple
+from typing import Optional
+from typing import Tuple
+
+from past.builtins import unicode
+
+from apache_beam import BeamJarExpansionService
+from apache_beam import ExternalTransform
+from apache_beam import NamedTupleBasedPayloadBuilder
+
+__all__ = [
+    'WriteToKinesis',
+    'ReadDataFromKinesis',
+    'InitialPositionInStream',
+    'WatermarkPolicy',
+]
+
+
+def default_io_expansion_service():
+  return BeamJarExpansionService(
+      'sdks:java:io:kinesis:expansion-service:shadowJar')
+
+
+WriteToKinesisSchema = NamedTuple(
+    'WriteToKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('partition_key', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('producer_properties', Optional[List[Tuple[unicode, unicode]]]),
+    ],
+)
+
+
+class WriteToKinesis(ExternalTransform):
+  """
+    An external PTransform which writes byte array stream to Amazon Kinesis.
+
+    Experimental; no backwards compatibility guarantees.
+  """
+  URN = 'beam:external:java:kinesis:write:v1'
+
+  def __init__(
+      self,
+      stream_name,
+      aws_access_key,
+      aws_secret_key,
+      region,
+      partition_key,
+      service_endpoint=None,
+      verify_certificate=None,
+      producer_properties=None,
+      expansion_service=None,
+  ):
+    """
+    Initializes a write operation to Kinesis.
+
+    :param stream_name: Kinesis stream name.
+    :param aws_access_key: Kinesis access key.
+    :param aws_secret_key: Kinesis access key secret.
+    :param region: AWS region. Example: 'us-east-1'.
+    :param service_endpoint: Kinesis service endpoint
+    :param verify_certificate: Enable or disable certificate verification.
+        Never set to False on production. True by default.
+    :param partition_key: Specify default partition key.
+    :param producer_properties: Specify the configuration properties for Kinesis
+        Producer Library (KPL) as List[KV[string, string]].
+        Example: [('CollectionMaxCount', '1000'), ('ConnectTimeout', '10000')]
+    :param expansion_service: The address (host:port) of the ExpansionService.
+    """
+    super(WriteToKinesis, self).__init__(
+        self.URN,
+        NamedTupleBasedPayloadBuilder(
+            WriteToKinesisSchema(
+                stream_name=stream_name,
+                aws_access_key=aws_access_key,
+                aws_secret_key=aws_secret_key,
+                region=region,
+                partition_key=partition_key,
+                service_endpoint=service_endpoint,
+                verify_certificate=verify_certificate,
+                producer_properties=producer_properties,
+            )),
+        expansion_service or default_io_expansion_service(),
+    )
+
+
+ReadFromKinesisSchema = NamedTuple(
+    'ReadFromKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('max_num_records', Optional[int]),
+        ('max_read_time', Optional[int]),
+        ('initial_position_in_stream', Optional[unicode]),
+        ('initial_timestamp_in_stream', Optional[int]),
+        ('request_records_limit', Optional[int]),
+        ('up_to_date_threshold', Optional[int]),
+        ('max_capacity_per_shard', Optional[int]),
+        ('watermark_policy', Optional[unicode]),
+        ('watermark_idle_duration_threshold', Optional[int]),
+        ('rate_limit', Optional[int]),
+    ],
+)
+
+
+class InitialPositionInStream:
+  LATEST = 'LATEST'
+  TRIM_HORIZON = 'TRIM_HORIZON'
+  AT_TIMESTAMP = 'AT_TIMESTAMP'
+
+
+class WatermarkPolicy:
+  PROCESSING_TYPE = 'PROCESSING_TYPE'
+  ARRIVAL_TIME = 'ARRIVAL_TIME'
+
+
+class ReadDataFromKinesis(ExternalTransform):
+  """
+    An external PTransform which reads byte array stream from Amazon Kinesis.
+
+    Experimental; no backwards compatibility guarantees.
+  """
+  URN = 'beam:external:java:kinesis:read_data:v1'
+
+  def __init__(
+      self,
+      stream_name,
+      aws_access_key,
+      aws_secret_key,
+      region,
+      service_endpoint=None,
+      verify_certificate=None,
+      max_num_records=None,
+      max_read_time=None,
+      initial_position_in_stream=None,
+      initial_timestamp_in_stream=None,
+      request_records_limit=None,
+      up_to_date_threshold=None,
+      max_capacity_per_shard=None,
+      watermark_policy=None,
+      watermark_idle_duration_threshold=None,
+      rate_limit=None,
+      expansion_service=None,
+  ):
+    """
+    Initializes a read operation from Kinesis.
+
+    :param stream_name: Kinesis stream name.
+    :param aws_access_key: Kinesis access key.
+    :param aws_secret_key: Kinesis access key secret.
+    :param region: AWS region. Example: 'us-east-1'.
+    :param service_endpoint: Kinesis service endpoint
+    :param verify_certificate: Enable or disable certificate verification.
+        Never set to False on production. True by default.
+    :param max_num_records: Specifies to read at most a given number of records.
+        Must be greater than 0.
+    :param max_read_time: Specifies to read records during x seconds.
+    :param initial_timestamp_in_stream: Specify reading beginning at the given
+        timestamp in seconds. Must be in the past.
+    :param initial_position_in_stream: Specify reading from some initial
+        position in stream. Possible values:
+        LATEST - Start after the most recent data record (fetch new data).
+        TRIM_HORIZON - Start from the oldest available data record.
+        AT_TIMESTAMP - Start from the record at or after the specified
+        server-side timestamp.
+    :param request_records_limit: Specifies the maximum number of records in
+        GetRecordsResult returned by GetRecords call which is limited by 10K
+        records. If should be adjusted according to average size of data record
+        to prevent shard overloading. More at:
+        docs.aws.amazon.com/kinesis/latest/APIReference/API_GetRecords.html
+    :param up_to_date_threshold: Specifies how late in seconds records consumed
+        by this source can be to still be considered on time. Defaults to zero.
+    :param max_capacity_per_shard: Specifies the maximum number of messages per
+        one shard. Defaults to 10'000.
+    :param watermark_policy: Specifies the watermark policy. Possible values:
+        PROCESSING_TYPE, ARRIVAL_TIME. Defaults to ARRIVAL_TIME.
+    :param watermark_idle_duration_threshold: Use only when watermark policy is
+        ARRIVAL_TIME. Denotes the duration for which the watermark can be idle.
+        Passed in seconds.
+    :param rate_limit: Sets fixed rate policy for given seconds value. By
+        default there is no rate limit.
+    :param expansion_service: The address (host:port) of the ExpansionService.
+    """
+    if watermark_policy:
+      assert watermark_policy == WatermarkPolicy.ARRIVAL_TIME or\
+             watermark_policy == WatermarkPolicy.PROCESSING_TYPE
+
+    if initial_position_in_stream:
+      i = initial_position_in_stream
+      assert i == InitialPositionInStream.AT_TIMESTAMP or \
+             i == InitialPositionInStream.LATEST or \
+             i == InitialPositionInStream.TRIM_HORIZON
+
+    if request_records_limit:
+      assert 0 < request_records_limit <= 10000
+
+    if initial_timestamp_in_stream:
+      assert initial_timestamp_in_stream < time.time()

Review comment:
       Yeah I think we should prefer millis, since that more closely maps to Instant. It looks like millis is the precision supported by Kinesis based on https://docs.aws.amazon.com/kinesis/latest/APIReference/API_StartingPosition.html




----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-665038984






----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **increase** coverage by `0.02%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   + Coverage   40.22%   40.24%   +0.02%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    53722      +53     
   ==========================================
   + Hits        21587    21621      +34     
   - Misses      32082    32101      +19     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   | [...beam/runners/interactive/background\_caching\_job.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9iYWNrZ3JvdW5kX2NhY2hpbmdfam9iLnB5) | `25.92% <0.00%> (+0.68%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **decrease** coverage by `0.12%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   - Coverage   40.22%   40.10%   -0.13%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    54025     +356     
   ==========================================
   + Hits        21587    21665      +78     
   - Misses      32082    32360     +278     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   | [.../runners/portability/fn\_api\_runner/translations.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3RyYW5zbGF0aW9ucy5weQ==) | `14.00% <0.00%> (+0.22%)` | :arrow_up: |
   | [...beam/runners/interactive/background\_caching\_job.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9iYWNrZ3JvdW5kX2NhY2hpbmdfam9iLnB5) | `25.92% <0.00%> (+0.68%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683609297


   Run Typescript 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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666279110


   @TheNeuralBit Sorry for lots of messages but every day brings something new.
   
   I've managed to modify existing KinesisIOIT to work with localstack (I'll push it in a separate PR).
   
   I've also managed to run cross-language read test. But it required putting this code anywhere in harness code, which is definitely not any solution because it affects whole beam:
   ```
   System.setProperty("com.amazonaws.sdk.disableCertChecking", "true");
   System.setProperty("com.amazonaws.sdk.disableCbor", "true");
   ```
   
   I have no idea how to dynamically inject these except for creating custom beam java sdk docker image with modified harness code especially for this test. I don't think it's an acceptable solution though. Do you possibly have a better idea?
   
   If this is the only thing we could do I'd just skip read test and just execute only write instead in the 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] piotr-szuberski removed a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski removed a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666361077


   Run Python 3.8 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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r468391526



##########
File path: sdks/python/apache_beam/io/kinesis.py
##########
@@ -0,0 +1,317 @@
+#
+# 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.
+#
+
+"""PTransforms for supporting Kinesis streaming in Python pipelines.
+
+  These transforms are currently supported by Beam Flink and Spark portable
+  runners.
+
+  **Setup**
+
+  Transforms provided in this module are cross-language transforms
+  implemented in the Beam Java SDK. During the pipeline construction, Python SDK
+  will connect to a Java expansion service to expand these transforms.
+  To facilitate this, a small amount of setup is needed before using these
+  transforms in a Beam Python pipeline.
+
+  There are several ways to setup cross-language Kinesis transforms.
+
+  * Option 1: use the default expansion service
+  * Option 2: specify a custom expansion service
+
+  See below for details regarding each of these options.
+
+  *Option 1: Use the default expansion service*
+
+  This is the recommended and easiest setup option for using Python Kinesis
+  transforms. This option is only available for Beam 2.24.0 and later.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Install Java runtime in the computer from where the pipeline is constructed
+    and make sure that 'java' command is available.
+
+  In this option, Python SDK will either download (for released Beam version) or
+  build (when running from a Beam Git clone) a expansion service jar and use
+  that to expand transforms. Currently Kinesis transforms use the
+  'beam-sdks-java-io-kinesis-expansion-service' jar for this purpose.
+
+  *Option 2: specify a custom expansion service*
+
+  In this option, you startup your own expansion service and provide that as
+  a parameter when using the transforms provided in this module.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Startup your own expansion service.
+  * Update your pipeline to provide the expansion service address when
+    initiating Kinesis transforms provided in this module.
+
+  Flink Users can use the built-in Expansion Service of the Flink Runner's
+  Job Server. If you start Flink's Job Server, the expansion service will be
+  started on port 8097. For a different address, please set the
+  expansion_service parameter.
+
+  **More information**
+
+  For more information regarding cross-language transforms see:
+  - https://beam.apache.org/roadmap/portability/
+
+  For more information specific to Flink runner see:
+  - https://beam.apache.org/documentation/runners/flink/
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import time
+from typing import List
+from typing import NamedTuple
+from typing import Optional
+from typing import Tuple
+
+from past.builtins import unicode
+
+from apache_beam import BeamJarExpansionService
+from apache_beam import ExternalTransform
+from apache_beam import NamedTupleBasedPayloadBuilder
+
+__all__ = [
+    'WriteToKinesis',
+    'ReadDataFromKinesis',
+    'InitialPositionInStream',
+    'WatermarkPolicy',
+]
+
+
+def default_io_expansion_service():
+  return BeamJarExpansionService(
+      'sdks:java:io:kinesis:expansion-service:shadowJar')
+
+
+WriteToKinesisSchema = NamedTuple(
+    'WriteToKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('partition_key', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('producer_properties', Optional[List[Tuple[unicode, unicode]]]),
+    ],
+)
+
+
+class WriteToKinesis(ExternalTransform):
+  """
+    An external PTransform which writes byte array stream to Amazon Kinesis.
+
+    Experimental; no backwards compatibility guarantees.
+  """
+  URN = 'beam:external:java:kinesis:write:v1'
+
+  def __init__(
+      self,
+      stream_name,
+      aws_access_key,
+      aws_secret_key,
+      region,
+      partition_key,
+      service_endpoint=None,
+      verify_certificate=None,
+      producer_properties=None,
+      expansion_service=None,
+  ):
+    """
+    Initializes a write operation to Kinesis.
+
+    :param stream_name: Kinesis stream name.
+    :param aws_access_key: Kinesis access key.
+    :param aws_secret_key: Kinesis access key secret.
+    :param region: AWS region. Example: 'us-east-1'.
+    :param service_endpoint: Kinesis service endpoint
+    :param verify_certificate: Enable or disable certificate verification.
+        Never set to False on production. True by default.
+    :param partition_key: Specify default partition key.
+    :param producer_properties: Specify the configuration properties for Kinesis
+        Producer Library (KPL) as List[KV[string, string]].
+        Example: [('CollectionMaxCount', '1000'), ('ConnectTimeout', '10000')]

Review comment:
       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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r459260343



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -561,6 +569,41 @@ public Read withMaxCapacityPerShard(Integer maxCapacity) {
     }
   }
 
+  /**
+   * A {@link PTransform} to read from Kinesis stream. Similar to {@link KinesisIO.Read}, but
+   * removes Kinesis metatdata and returns a {@link PCollection} of {@link byte[]}. See {@link
+   * KinesisIO} for more information on usage and configuration of reader.
+   */
+  public static class TypedWithoutMetadata extends PTransform<PBegin, PCollection<byte[]>> {

Review comment:
       I followed the code in KafkaIO where TypedWithoutMetadata is used. ReadWithoutMetadata tells indeed much more.




----------------------------------------------------------------
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] TheNeuralBit commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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


   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] TheNeuralBit commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisTransformRegistrar.java
##########
@@ -0,0 +1,268 @@
+/*
+ * 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.
+ */
+package org.apache.beam.sdk.io.kinesis;
+
+import com.amazonaws.regions.Regions;
+import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream;
+import com.google.auto.service.AutoService;
+import java.util.Map;
+import java.util.Properties;
+import javax.annotation.Nullable;

Review comment:
       We should prefer the checker framework Nullable annotation, `org.checkerframework.checker.nullness.qual.Nullable`

##########
File path: sdks/python/apache_beam/io/external/xlang_kinesisio_it_test.py
##########
@@ -0,0 +1,308 @@
+#
+# 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 Python cross-language pipelines for Java KinesisIO.
+
+If you want to run the tests on localstack then run it just with pipeline
+options.
+
+To test it on a real AWS account you need to pass some additional params, e.g.:
+python setup.py nosetests \
+--tests=apache_beam.io.external.xlang_kinesisio_it_test \
+--test-pipeline-options="
+  --use_real_aws
+  --aws_kinesis_stream=<STREAM_NAME>
+  --aws_access_key=<AWS_ACCESS_KEY>
+  --aws_secret_key=<AWS_SECRET_KEY>
+  --aws_region=<AWS_REGION>
+  --runner=FlinkRunner"
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import argparse
+import logging
+import time
+import unittest
+import uuid
+
+import apache_beam as beam
+from apache_beam.io.kinesis import InitialPositionInStream
+from apache_beam.io.kinesis import ReadDataFromKinesis
+from apache_beam.io.kinesis import WriteToKinesis
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import StandardOptions
+from apache_beam.testing.test_pipeline import TestPipeline
+from apache_beam.testing.util import assert_that
+from apache_beam.testing.util import equal_to
+
+# pylint: disable=wrong-import-order, wrong-import-position, ungrouped-imports
+try:
+  import boto3
+except ImportError:
+  boto3 = None
+
+try:
+  from testcontainers.core.container import DockerContainer
+except ImportError:
+  DockerContainer = None
+# pylint: enable=wrong-import-order, wrong-import-position, ungrouped-imports
+
+LOCALSTACK_VERSION = '0.11.3'
+NUM_RECORDS = 10
+NOW = time.time()
+RECORD = b'record' + str(uuid.uuid4()).encode()
+
+
+@unittest.skipUnless(DockerContainer, 'testcontainers is not installed.')
+@unittest.skipUnless(boto3, 'boto3 is not installed.')
+@unittest.skipUnless(
+    TestPipeline().get_pipeline_options().view_as(StandardOptions).runner,
+    'Do not run this test on precommit suites.')
+class CrossLanguageKinesisIOTest(unittest.TestCase):
+  @unittest.skipUnless(
+      TestPipeline().get_option('aws_kinesis_stream'),
+      'Cannot test on real aws without pipeline options provided')
+  def test_kinesis_io_roundtrip(self):
+    # TODO: enable this test for localstack once BEAM-10664 is resolved
+    self.run_kinesis_write()
+    self.run_kinesis_read()
+
+  @unittest.skipIf(
+      TestPipeline().get_option('aws_kinesis_stream'),
+      'Do not test on localstack when pipeline options were provided')
+  def test_kinesis_write(self):
+    # TODO: remove this test once BEAM-10664 is resolved
+    self.run_kinesis_write()
+    records = self.kinesis_helper.read_from_stream(self.aws_kinesis_stream)
+    self.assertEqual(
+        sorted(records),
+        sorted([RECORD + str(i).encode() for i in range(NUM_RECORDS)]))
+
+  def run_kinesis_write(self):
+    with TestPipeline(options=PipelineOptions(self.pipeline_args)) as p:
+      p.not_use_test_runner_api = True
+      _ = (
+          p
+          | 'Impulse' >> beam.Impulse()
+          | 'Generate' >> beam.FlatMap(lambda x: range(NUM_RECORDS))  # pylint: disable=range-builtin-not-iterating
+          | 'Map to bytes' >>
+          beam.Map(lambda x: RECORD + str(x).encode()).with_output_types(bytes)
+          | 'WriteToKinesis' >> WriteToKinesis(
+              stream_name=self.aws_kinesis_stream,
+              aws_access_key=self.aws_access_key,
+              aws_secret_key=self.aws_secret_key,
+              region=self.aws_region,
+              service_endpoint=self.aws_service_endpoint,
+              verify_certificate=(not self.use_localstack),
+              partition_key='1',
+          ))
+
+  def run_kinesis_read(self):
+    records = [RECORD + str(i).encode() for i in range(NUM_RECORDS)]
+
+    with TestPipeline(options=PipelineOptions(self.pipeline_args)) as p:
+      result = (
+          p
+          | 'ReadFromKinesis' >> ReadDataFromKinesis(
+              stream_name=self.aws_kinesis_stream,
+              aws_access_key=self.aws_access_key,
+              aws_secret_key=self.aws_secret_key,
+              region=self.aws_region,
+              service_endpoint=self.aws_service_endpoint,
+              verify_certificate=not self.use_localstack,
+              max_num_records=NUM_RECORDS,
+              max_read_time=300,  # 5min
+              initial_position_in_stream=InitialPositionInStream.AT_TIMESTAMP,
+              initial_timestamp_in_stream=int(NOW),
+          ).with_output_types(bytes))
+      assert_that(result, equal_to(records))
+
+  def set_localstack(self):
+    self.localstack = DockerContainer('localstack/localstack:{}'
+                                      .format(LOCALSTACK_VERSION))\
+      .with_env('SERVICES', 'kinesis')\
+      .with_env('KINESIS_PORT', '4568')\
+      .with_env('USE_SSL', 'true')\
+      .with_exposed_ports(4568)\
+      .with_volume_mapping('/var/run/docker.sock', '/var/run/docker.sock', 'rw')
+
+    # Repeat if ReadTimeout is raised.
+    for i in range(4):
+      try:
+        self.localstack.start()
+        break
+      except Exception as e:  # pylint: disable=bare-except
+        if i == 3:
+          logging.error('Could not initialize localstack container')
+          raise e
+
+    self.aws_service_endpoint = 'https://{}:{}'.format(
+        self.localstack.get_container_host_ip(),
+        self.localstack.get_exposed_port('4568'),
+    )
+
+  def setUp(self):
+    parser = argparse.ArgumentParser()
+
+    parser.add_argument(
+        '--aws_kinesis_stream',
+        default='beam_kinesis_xlang',
+        help='Kinesis stream name',
+    )
+    parser.add_argument(
+        '--aws_access_key',
+        default='accesskey',
+        help=('Aws access key'),
+    )
+    parser.add_argument(
+        '--aws_secret_key',
+        default='secretkey',
+        help='Aws secret key',
+    )
+    parser.add_argument(
+        '--aws_region',
+        default='us-east-1',
+        help='Aws region',
+    )
+    parser.add_argument(
+        '--aws_service_endpoint',
+        default=None,
+        help='Url to external aws endpoint',
+    )
+    parser.add_argument(
+        '--use_real_aws',
+        default=False,
+        dest='use_real_aws',
+        action='store_true',
+        help='Flag whether to use real aws for the tests purpose',
+    )
+    parser.add_argument(
+        '--expansion_service',
+        help='Url to externally launched expansion service.',
+    )
+
+    pipeline = TestPipeline()
+    argv = pipeline.get_full_options_as_args()
+
+    known_args, self.pipeline_args = parser.parse_known_args(argv)
+
+    self.aws_kinesis_stream = known_args.aws_kinesis_stream
+    self.aws_access_key = known_args.aws_access_key
+    self.aws_secret_key = known_args.aws_secret_key
+    self.aws_region = known_args.aws_region
+    self.aws_service_endpoint = known_args.aws_service_endpoint
+    self.use_localstack = not known_args.use_real_aws
+    self.expansion_service = known_args.expansion_service
+
+    if self.use_localstack:
+      self.set_localstack()
+
+    self.kinesis_helper = KinesisHelper(
+        self.aws_access_key,
+        self.aws_secret_key,
+        self.aws_region,
+        self.aws_service_endpoint.replace('https', 'http')

Review comment:
       Is this necessary?




----------------------------------------------------------------
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] TheNeuralBit commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

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



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisReadTransformRegistrar.java
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.
+ */
+package org.apache.beam.sdk.io.kinesis;
+
+import com.amazonaws.regions.Regions;
+import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream;
+import com.google.auto.service.AutoService;
+import java.util.Map;
+import org.apache.beam.sdk.annotations.Experimental;
+import org.apache.beam.sdk.annotations.Experimental.Kind;
+import org.apache.beam.sdk.expansion.ExternalTransformRegistrar;
+import org.apache.beam.sdk.transforms.ExternalTransformBuilder;
+import org.apache.beam.sdk.transforms.PTransform;
+import org.apache.beam.sdk.values.PBegin;
+import org.apache.beam.sdk.values.PCollection;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableMap;
+import org.joda.time.Duration;
+import org.joda.time.Instant;
+
+/** Exposes {@link KinesisIO.Read} as an external transform for cross-language usage. */
+@Experimental(Kind.PORTABILITY)
+@AutoService(ExternalTransformRegistrar.class)
+public class KinesisReadTransformRegistrar implements ExternalTransformRegistrar {

Review comment:
       I think it would be cleaner just to have a single registrar with inner classes for all the builders and the configuration classes. WDYT?

##########
File path: sdks/java/io/kinesis/expansion-service/build.gradle
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+
+apply plugin: 'org.apache.beam.module'
+apply plugin: 'application'
+mainClassName = "org.apache.beam.sdk.expansion.service.ExpansionService"
+
+applyJavaNature(enableChecker:false,

Review comment:
       Are you able to run the nullability checker, or does it complain about underlying issues in `:sdks:java:io:kinesis`? We should prefer enabling the nullability checker in new packages if possible since there's an effort to enable the checker throughout Beam (BEAM-10402).

##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -561,6 +569,41 @@ public Read withMaxCapacityPerShard(Integer maxCapacity) {
     }
   }
 
+  /**
+   * A {@link PTransform} to read from Kinesis stream. Similar to {@link KinesisIO.Read}, but
+   * removes Kinesis metatdata and returns a {@link PCollection} of {@link byte[]}. See {@link
+   * KinesisIO} for more information on usage and configuration of reader.
+   */
+  public static class TypedWithoutMetadata extends PTransform<PBegin, PCollection<byte[]>> {

Review comment:
       Also bikesheddy comment: I'm not sure about the name TypedWithoutMetadata, should it be ReadWithoutMetadata?

##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -561,6 +569,41 @@ public Read withMaxCapacityPerShard(Integer maxCapacity) {
     }
   }
 
+  /**
+   * A {@link PTransform} to read from Kinesis stream. Similar to {@link KinesisIO.Read}, but
+   * removes Kinesis metatdata and returns a {@link PCollection} of {@link byte[]}. See {@link
+   * KinesisIO} for more information on usage and configuration of reader.
+   */
+  public static class TypedWithoutMetadata extends PTransform<PBegin, PCollection<byte[]>> {

Review comment:
       We should file a jira for making `KinesisIO.Read` (with metadata) available. I think this would be possible if we register a schema for `KinesisRecord`, but it won't work cross-language until we have portable support for datetimes (i.e. BEAM-7554 will be a blocker). Also maybe this one should use a URN like "read_without_metadata" or "read_data_only" so we can use "read" in the future.

##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -536,6 +537,13 @@ public Read withMaxCapacityPerShard(Integer maxCapacity) {
       return toBuilder().setMaxCapacityPerShard(maxCapacity).build();
     }
 
+    /**
+     * Returns a {@link PTransform} for PCollection of {@link byte[]}, dropping Kinesis metatdata.
+     */
+    public PTransform<PBegin, PCollection<byte[]>> withoutMetadata() {
+      return new TypedWithoutMetadata(this);
+    }

Review comment:
       I think it will be confusing to have this function here, since every other `with...` method returns a `Read` class. A user might expect to be able to do:
   
   ```
   KinesisIO.read()
      .withoutMetadata()
      .withMaxCapacityPerShard(x)
   ```
   
   Instead we should add a `KinesisIO.readWithoutMetadata()` that returns something like `TypedWithoutMetadata` with all the same configuration parameters as `Read`. You might take a look at how this is handled in `PubsubIO` for inspiration, I think it has a similar problem where it supports multiple different output types, but we want them all to share some configuration parameters. It looks like the way it's handled there is with a Read<T> type, and you pass in a function for converting PubsubMessage to T.
   
   Alternatively, you could move the logic to extract the payload into the external transform builder, rather than adding the interface here (I think there's value in adding it here though if you want to take it on).




----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **decrease** coverage by `0.12%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   - Coverage   40.22%   40.10%   -0.13%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    54025     +356     
   ==========================================
   + Hits        21587    21665      +78     
   - Misses      32082    32360     +278     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   | [.../runners/portability/fn\_api\_runner/translations.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3RyYW5zbGF0aW9ucy5weQ==) | `14.00% <0.00%> (+0.22%)` | :arrow_up: |
   | [...beam/runners/interactive/background\_caching\_job.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9iYWNrZ3JvdW5kX2NhY2hpbmdfam9iLnB5) | `25.92% <0.00%> (+0.68%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] mwalenia merged pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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


   


----------------------------------------------------------------
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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r469040295



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisTransformRegistrar.java
##########
@@ -0,0 +1,268 @@
+/*
+ * 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.
+ */
+package org.apache.beam.sdk.io.kinesis;
+
+import com.amazonaws.regions.Regions;
+import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream;
+import com.google.auto.service.AutoService;
+import java.util.Map;
+import java.util.Properties;
+import javax.annotation.Nullable;

Review comment:
       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] yuanhunglo commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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


   I tried that and received this error. I'll wait till 2.25.0 release to try again. Thanks!
   
   > _InactiveRpcError: <_InactiveRpcError of RPC that terminated with:
   	status = StatusCode.UNAVAILABLE
   	details = "failed to connect to all addresses"
   	debug_error_string = "{"created":"@1602094791.569834934","description":"Failed to pick subchannel","file":"src/core/ext/filters/client_channel/client_channel.cc","file_line":4133,"referenced_errors":[{"created":"@1602094791.569831898","description":"failed to connect to all addresses","file":"src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc","file_line":397,"grpc_status":14}]}"
   


----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666363032


   @TheNeuralBit Forget what I said. I've just found that the expansion-service has an indirect dependency on harness and I managed to set test properties in gradle file. Now both read and write seem to work. I think that this PR is ready to review now.
   I don't know why python linter failed on an unrelated file: spark_uber_jar_job_server_test


----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666408845


   Run Python 3.8 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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r459246455



##########
File path: sdks/java/io/kinesis/expansion-service/build.gradle
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+
+apply plugin: 'org.apache.beam.module'
+apply plugin: 'application'
+mainClassName = "org.apache.beam.sdk.expansion.service.ExpansionService"
+
+applyJavaNature(enableChecker:false,

Review comment:
       I enabled it and it doesn't seem to have any negative effect. I'm doing this change in Write PR (I'll rebase this one onto these changes so this time there will be no force-pushing like it was in jdbc 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.

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



[GitHub] [beam] codecov[bot] commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **increase** coverage by `0.02%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   + Coverage   40.22%   40.24%   +0.02%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    53717      +48     
   ==========================================
   + Hits        21587    21619      +32     
   - Misses      32082    32098      +16     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666350605


   Run Python 3.8 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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666472634


   Run Python 3.8 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] TheNeuralBit commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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


   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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-665688027






----------------------------------------------------------------
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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666363032






----------------------------------------------------------------
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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r459245205



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisReadTransformRegistrar.java
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.
+ */
+package org.apache.beam.sdk.io.kinesis;
+
+import com.amazonaws.regions.Regions;
+import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream;
+import com.google.auto.service.AutoService;
+import java.util.Map;
+import org.apache.beam.sdk.annotations.Experimental;
+import org.apache.beam.sdk.annotations.Experimental.Kind;
+import org.apache.beam.sdk.expansion.ExternalTransformRegistrar;
+import org.apache.beam.sdk.transforms.ExternalTransformBuilder;
+import org.apache.beam.sdk.transforms.PTransform;
+import org.apache.beam.sdk.values.PBegin;
+import org.apache.beam.sdk.values.PCollection;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableMap;
+import org.joda.time.Duration;
+import org.joda.time.Instant;
+
+/** Exposes {@link KinesisIO.Read} as an external transform for cross-language usage. */
+@Experimental(Kind.PORTABILITY)
+@AutoService(ExternalTransformRegistrar.class)
+public class KinesisReadTransformRegistrar implements ExternalTransformRegistrar {

Review comment:
       I much prefer extracting as much code as possible but since everything is in one package in Kinesis then it makes sense.




----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683591758


   > Please resolve the conflict.
   
   @chamikaramj 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] TheNeuralBit commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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


   Run Portable_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] TheNeuralBit commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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



##########
File path: sdks/python/apache_beam/io/kinesis.py
##########
@@ -0,0 +1,317 @@
+#
+# 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.
+#
+
+"""PTransforms for supporting Kinesis streaming in Python pipelines.
+
+  These transforms are currently supported by Beam Flink and Spark portable
+  runners.
+
+  **Setup**
+
+  Transforms provided in this module are cross-language transforms
+  implemented in the Beam Java SDK. During the pipeline construction, Python SDK
+  will connect to a Java expansion service to expand these transforms.
+  To facilitate this, a small amount of setup is needed before using these
+  transforms in a Beam Python pipeline.
+
+  There are several ways to setup cross-language Kinesis transforms.
+
+  * Option 1: use the default expansion service
+  * Option 2: specify a custom expansion service
+
+  See below for details regarding each of these options.
+
+  *Option 1: Use the default expansion service*
+
+  This is the recommended and easiest setup option for using Python Kinesis
+  transforms. This option is only available for Beam 2.24.0 and later.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Install Java runtime in the computer from where the pipeline is constructed
+    and make sure that 'java' command is available.
+
+  In this option, Python SDK will either download (for released Beam version) or
+  build (when running from a Beam Git clone) a expansion service jar and use
+  that to expand transforms. Currently Kinesis transforms use the
+  'beam-sdks-java-io-kinesis-expansion-service' jar for this purpose.
+
+  *Option 2: specify a custom expansion service*
+
+  In this option, you startup your own expansion service and provide that as
+  a parameter when using the transforms provided in this module.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Startup your own expansion service.
+  * Update your pipeline to provide the expansion service address when
+    initiating Kinesis transforms provided in this module.
+
+  Flink Users can use the built-in Expansion Service of the Flink Runner's
+  Job Server. If you start Flink's Job Server, the expansion service will be
+  started on port 8097. For a different address, please set the
+  expansion_service parameter.
+
+  **More information**
+
+  For more information regarding cross-language transforms see:
+  - https://beam.apache.org/roadmap/portability/
+
+  For more information specific to Flink runner see:
+  - https://beam.apache.org/documentation/runners/flink/
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import time
+from typing import List
+from typing import NamedTuple
+from typing import Optional
+from typing import Tuple
+
+from past.builtins import unicode
+
+from apache_beam import BeamJarExpansionService
+from apache_beam import ExternalTransform
+from apache_beam import NamedTupleBasedPayloadBuilder
+
+__all__ = [
+    'WriteToKinesis',
+    'ReadDataFromKinesis',
+    'InitialPositionInStream',
+    'WatermarkPolicy',
+]
+
+
+def default_io_expansion_service():
+  return BeamJarExpansionService(
+      'sdks:java:io:kinesis:expansion-service:shadowJar')
+
+
+WriteToKinesisSchema = NamedTuple(
+    'WriteToKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('partition_key', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('producer_properties', Optional[List[Tuple[unicode, unicode]]]),
+    ],
+)
+
+
+class WriteToKinesis(ExternalTransform):
+  """
+    An external PTransform which writes byte array stream to Amazon Kinesis.
+
+    Experimental; no backwards compatibility guarantees.
+  """
+  URN = 'beam:external:java:kinesis:write:v1'
+
+  def __init__(
+      self,
+      stream_name,
+      aws_access_key,
+      aws_secret_key,
+      region,
+      partition_key,
+      service_endpoint=None,
+      verify_certificate=None,
+      producer_properties=None,
+      expansion_service=None,
+  ):
+    """
+    Initializes a write operation to Kinesis.
+
+    :param stream_name: Kinesis stream name.
+    :param aws_access_key: Kinesis access key.
+    :param aws_secret_key: Kinesis access key secret.
+    :param region: AWS region. Example: 'us-east-1'.
+    :param service_endpoint: Kinesis service endpoint
+    :param verify_certificate: Enable or disable certificate verification.
+        Never set to False on production. True by default.
+    :param partition_key: Specify default partition key.
+    :param producer_properties: Specify the configuration properties for Kinesis
+        Producer Library (KPL) as List[KV[string, string]].
+        Example: [('CollectionMaxCount', '1000'), ('ConnectTimeout', '10000')]
+    :param expansion_service: The address (host:port) of the ExpansionService.
+    """
+    super(WriteToKinesis, self).__init__(
+        self.URN,
+        NamedTupleBasedPayloadBuilder(
+            WriteToKinesisSchema(
+                stream_name=stream_name,
+                aws_access_key=aws_access_key,
+                aws_secret_key=aws_secret_key,
+                region=region,
+                partition_key=partition_key,
+                service_endpoint=service_endpoint,
+                verify_certificate=verify_certificate,
+                producer_properties=producer_properties,
+            )),
+        expansion_service or default_io_expansion_service(),
+    )
+
+
+ReadFromKinesisSchema = NamedTuple(
+    'ReadFromKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('max_num_records', Optional[int]),
+        ('max_read_time', Optional[int]),
+        ('initial_position_in_stream', Optional[unicode]),
+        ('initial_timestamp_in_stream', Optional[int]),
+        ('request_records_limit', Optional[int]),
+        ('up_to_date_threshold', Optional[int]),
+        ('max_capacity_per_shard', Optional[int]),
+        ('watermark_policy', Optional[unicode]),
+        ('watermark_idle_duration_threshold', Optional[int]),
+        ('rate_limit', Optional[int]),
+    ],
+)
+
+
+class InitialPositionInStream:
+  LATEST = 'LATEST'
+  TRIM_HORIZON = 'TRIM_HORIZON'
+  AT_TIMESTAMP = 'AT_TIMESTAMP'
+
+
+class WatermarkPolicy:
+  PROCESSING_TYPE = 'PROCESSING_TYPE'
+  ARRIVAL_TIME = 'ARRIVAL_TIME'
+
+
+class ReadDataFromKinesis(ExternalTransform):
+  """
+    An external PTransform which reads byte array stream from Amazon Kinesis.
+
+    Experimental; no backwards compatibility guarantees.
+  """
+  URN = 'beam:external:java:kinesis:read_data:v1'
+
+  def __init__(
+      self,
+      stream_name,
+      aws_access_key,
+      aws_secret_key,
+      region,
+      service_endpoint=None,
+      verify_certificate=None,
+      max_num_records=None,
+      max_read_time=None,
+      initial_position_in_stream=None,
+      initial_timestamp_in_stream=None,
+      request_records_limit=None,
+      up_to_date_threshold=None,
+      max_capacity_per_shard=None,
+      watermark_policy=None,
+      watermark_idle_duration_threshold=None,
+      rate_limit=None,
+      expansion_service=None,
+  ):
+    """
+    Initializes a read operation from Kinesis.
+
+    :param stream_name: Kinesis stream name.
+    :param aws_access_key: Kinesis access key.
+    :param aws_secret_key: Kinesis access key secret.
+    :param region: AWS region. Example: 'us-east-1'.
+    :param service_endpoint: Kinesis service endpoint
+    :param verify_certificate: Enable or disable certificate verification.
+        Never set to False on production. True by default.
+    :param max_num_records: Specifies to read at most a given number of records.
+        Must be greater than 0.
+    :param max_read_time: Specifies to read records during x seconds.
+    :param initial_timestamp_in_stream: Specify reading beginning at the given
+        timestamp in seconds. Must be in the past.
+    :param initial_position_in_stream: Specify reading from some initial
+        position in stream. Possible values:
+        LATEST - Start after the most recent data record (fetch new data).
+        TRIM_HORIZON - Start from the oldest available data record.
+        AT_TIMESTAMP - Start from the record at or after the specified
+        server-side timestamp.
+    :param request_records_limit: Specifies the maximum number of records in
+        GetRecordsResult returned by GetRecords call which is limited by 10K
+        records. If should be adjusted according to average size of data record
+        to prevent shard overloading. More at:
+        docs.aws.amazon.com/kinesis/latest/APIReference/API_GetRecords.html
+    :param up_to_date_threshold: Specifies how late in seconds records consumed
+        by this source can be to still be considered on time. Defaults to zero.
+    :param max_capacity_per_shard: Specifies the maximum number of messages per
+        one shard. Defaults to 10'000.
+    :param watermark_policy: Specifies the watermark policy. Possible values:
+        PROCESSING_TYPE, ARRIVAL_TIME. Defaults to ARRIVAL_TIME.
+    :param watermark_idle_duration_threshold: Use only when watermark policy is
+        ARRIVAL_TIME. Denotes the duration for which the watermark can be idle.
+        Passed in seconds.
+    :param rate_limit: Sets fixed rate policy for given seconds value. By
+        default there is no rate limit.
+    :param expansion_service: The address (host:port) of the ExpansionService.
+    """
+    if watermark_policy:
+      assert watermark_policy == WatermarkPolicy.ARRIVAL_TIME or\
+             watermark_policy == WatermarkPolicy.PROCESSING_TYPE
+
+    if initial_position_in_stream:
+      i = initial_position_in_stream
+      assert i == InitialPositionInStream.AT_TIMESTAMP or \
+             i == InitialPositionInStream.LATEST or \
+             i == InitialPositionInStream.TRIM_HORIZON
+
+    if request_records_limit:
+      assert 0 < request_records_limit <= 10000
+
+    if initial_timestamp_in_stream:
+      assert initial_timestamp_in_stream < time.time()

Review comment:
       This worries me a little bit since the system clock could be wrong. Maybe this should be a warning instead?

##########
File path: sdks/python/apache_beam/io/kinesis.py
##########
@@ -0,0 +1,317 @@
+#
+# 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.
+#
+
+"""PTransforms for supporting Kinesis streaming in Python pipelines.
+
+  These transforms are currently supported by Beam Flink and Spark portable
+  runners.
+
+  **Setup**
+
+  Transforms provided in this module are cross-language transforms
+  implemented in the Beam Java SDK. During the pipeline construction, Python SDK
+  will connect to a Java expansion service to expand these transforms.
+  To facilitate this, a small amount of setup is needed before using these
+  transforms in a Beam Python pipeline.
+
+  There are several ways to setup cross-language Kinesis transforms.
+
+  * Option 1: use the default expansion service
+  * Option 2: specify a custom expansion service
+
+  See below for details regarding each of these options.
+
+  *Option 1: Use the default expansion service*
+
+  This is the recommended and easiest setup option for using Python Kinesis
+  transforms. This option is only available for Beam 2.24.0 and later.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Install Java runtime in the computer from where the pipeline is constructed
+    and make sure that 'java' command is available.
+
+  In this option, Python SDK will either download (for released Beam version) or
+  build (when running from a Beam Git clone) a expansion service jar and use
+  that to expand transforms. Currently Kinesis transforms use the
+  'beam-sdks-java-io-kinesis-expansion-service' jar for this purpose.
+
+  *Option 2: specify a custom expansion service*
+
+  In this option, you startup your own expansion service and provide that as
+  a parameter when using the transforms provided in this module.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Startup your own expansion service.
+  * Update your pipeline to provide the expansion service address when
+    initiating Kinesis transforms provided in this module.
+
+  Flink Users can use the built-in Expansion Service of the Flink Runner's
+  Job Server. If you start Flink's Job Server, the expansion service will be
+  started on port 8097. For a different address, please set the
+  expansion_service parameter.
+
+  **More information**
+
+  For more information regarding cross-language transforms see:
+  - https://beam.apache.org/roadmap/portability/
+
+  For more information specific to Flink runner see:
+  - https://beam.apache.org/documentation/runners/flink/
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import time
+from typing import List
+from typing import NamedTuple
+from typing import Optional
+from typing import Tuple
+
+from past.builtins import unicode
+
+from apache_beam import BeamJarExpansionService
+from apache_beam import ExternalTransform
+from apache_beam import NamedTupleBasedPayloadBuilder
+
+__all__ = [
+    'WriteToKinesis',
+    'ReadDataFromKinesis',
+    'InitialPositionInStream',
+    'WatermarkPolicy',
+]
+
+
+def default_io_expansion_service():
+  return BeamJarExpansionService(
+      'sdks:java:io:kinesis:expansion-service:shadowJar')
+
+
+WriteToKinesisSchema = NamedTuple(
+    'WriteToKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('partition_key', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('producer_properties', Optional[List[Tuple[unicode, unicode]]]),

Review comment:
       FYI if https://github.com/apache/beam/pull/12481 is merged first this can (and will have to) change to `Mapping[unicode, unicode]`

##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -295,14 +300,16 @@
   private static final int DEFAULT_NUM_RETRIES = 6;
 
   /** Returns a new {@link Read} transform for reading from Kinesis. */
-  public static Read read() {
-    return new AutoValue_KinesisIO_Read.Builder()
-        .setMaxNumRecords(Long.MAX_VALUE)
-        .setUpToDateThreshold(Duration.ZERO)
-        .setWatermarkPolicyFactory(WatermarkPolicyFactory.withArrivalTimePolicy())
-        .setRateLimitPolicyFactory(RateLimitPolicyFactory.withoutLimiter())
-        .setMaxCapacityPerShard(ShardReadersPool.DEFAULT_CAPACITY_PER_SHARD)
-        .build();
+  public static Read<KinesisRecord> read() {

Review comment:
       @lukecwik I suggested that Piotr make Read generic so that we can add `Read<byte[]> readData()` naturally, but now I'm wondering if this is a bad idea since it changes our Public API. Does this have a risk of breaking users?
   
   It looks like at least the way the method is used in our tests (`p.apply(KinesisIO.read())`) is unaffected.

##########
File path: sdks/python/apache_beam/io/external/xlang_kinesisio_it_test.py
##########
@@ -0,0 +1,304 @@
+#
+# 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 Python cross-language pipelines for Java KinesisIO.
+
+If you want to run the tests on localstack then run it just with pipeline
+options.
+
+To test it on a real AWS account you need to pass some additional params, e.g.:
+python setup.py nosetests \
+--tests=apache_beam.io.external.xlang_kinesisio_it_test \
+--test-pipeline-options="
+  --use_real_aws
+  --aws_kinesis_stream=<STREAM_NAME>
+  --aws_access_key=<AWS_ACCESS_KEY>
+  --aws_secret_key=<AWS_SECRET_KEY>
+  --aws_region=<AWS_REGION>
+  --runner=FlinkRunner"
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import argparse
+import logging
+import time
+import unittest
+import uuid
+
+import apache_beam as beam
+from apache_beam.io.kinesis import InitialPositionInStream
+from apache_beam.io.kinesis import ReadDataFromKinesis
+from apache_beam.io.kinesis import WriteToKinesis
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import StandardOptions
+from apache_beam.testing.test_pipeline import TestPipeline
+from apache_beam.testing.util import assert_that
+from apache_beam.testing.util import equal_to
+
+# pylint: disable=wrong-import-order, wrong-import-position, ungrouped-imports
+try:
+  import boto3
+except ImportError:
+  boto3 = None
+
+try:
+  from testcontainers.core.container import DockerContainer
+except ImportError:
+  DockerContainer = None
+# pylint: enable=wrong-import-order, wrong-import-position, ungrouped-imports
+
+LOCALSTACK_VERSION = '0.11.3'
+NUM_RECORDS = 10
+NOW = time.time()
+RECORD = b'record' + str(uuid.uuid4()).encode()
+
+
+@unittest.skipIf(DockerContainer is None, 'testcontainers is not installed.')
+@unittest.skipIf(boto3 is None, 'boto3 is not installed.')
+@unittest.skipIf(
+    TestPipeline().get_pipeline_options().view_as(StandardOptions).runner is
+    None,
+    'Do not run this test on precommit suites.',
+)
+class CrossLanguageKinesisIOTest(unittest.TestCase):
+  def test_kinesis_io(self):
+    self.run_kinesis_write()
+    # TODO: remove once BEAM-10664 is resolved
+    if not self.use_localstack:
+      self.run_kinesis_read_data()
+
+  def run_kinesis_write(self):
+    with TestPipeline(options=PipelineOptions(self.pipeline_args)) as p:
+      p.not_use_test_runner_api = True
+      _ = (
+          p
+          | 'Impulse' >> beam.Impulse()
+          | 'Generate' >> beam.FlatMap(lambda x: range(NUM_RECORDS))  # pylint: disable=range-builtin-not-iterating
+          | 'Map to bytes' >>
+          beam.Map(lambda x: RECORD + str(x).encode()).with_output_types(bytes)
+          | 'WriteToKinesis' >> WriteToKinesis(
+              stream_name=self.aws_kinesis_stream,
+              aws_access_key=self.aws_access_key,
+              aws_secret_key=self.aws_secret_key,
+              region=self.aws_region,
+              service_endpoint=self.aws_service_endpoint,
+              verify_certificate=(not self.use_localstack),
+              partition_key='1',
+          ))
+
+    # TODO: Remove once BEAM-10664 is resolved
+    if self.use_localstack:
+      records = self.kinesis_helper.read_from_stream(self.aws_kinesis_stream)
+      self.assertEqual(
+          sorted(records),
+          sorted([RECORD + str(i).encode() for i in range(NUM_RECORDS)]))

Review comment:
       I think it would be better if this had three tests:
   - test_kinesis_io_roundtrip (what you have now in the `self.use_localstack = False` case)
   - test_kinesis_write (what you have now in the `self.use_localstack = True` case)
   - test_kinesis_read (a new test that runs `run_kinesis_read_data` and injects the test data)
   
   test_kinesis_io_roundtrip and test_kinesis_read would be skipped when `self.use_localstack = True`
   If you'd rather not write up `test_kinesis_read` right now that could be left as a TODO for BEAM-10664. But I think we should at least separate out the two versions of test_kinesis_io into two distinct tests, so it's clear what is being tested.

##########
File path: sdks/python/apache_beam/io/kinesis.py
##########
@@ -0,0 +1,317 @@
+#
+# 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.
+#
+
+"""PTransforms for supporting Kinesis streaming in Python pipelines.
+
+  These transforms are currently supported by Beam Flink and Spark portable
+  runners.
+
+  **Setup**
+
+  Transforms provided in this module are cross-language transforms
+  implemented in the Beam Java SDK. During the pipeline construction, Python SDK
+  will connect to a Java expansion service to expand these transforms.
+  To facilitate this, a small amount of setup is needed before using these
+  transforms in a Beam Python pipeline.
+
+  There are several ways to setup cross-language Kinesis transforms.
+
+  * Option 1: use the default expansion service
+  * Option 2: specify a custom expansion service
+
+  See below for details regarding each of these options.
+
+  *Option 1: Use the default expansion service*
+
+  This is the recommended and easiest setup option for using Python Kinesis
+  transforms. This option is only available for Beam 2.24.0 and later.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Install Java runtime in the computer from where the pipeline is constructed
+    and make sure that 'java' command is available.
+
+  In this option, Python SDK will either download (for released Beam version) or
+  build (when running from a Beam Git clone) a expansion service jar and use
+  that to expand transforms. Currently Kinesis transforms use the
+  'beam-sdks-java-io-kinesis-expansion-service' jar for this purpose.
+
+  *Option 2: specify a custom expansion service*
+
+  In this option, you startup your own expansion service and provide that as
+  a parameter when using the transforms provided in this module.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Startup your own expansion service.
+  * Update your pipeline to provide the expansion service address when
+    initiating Kinesis transforms provided in this module.
+
+  Flink Users can use the built-in Expansion Service of the Flink Runner's
+  Job Server. If you start Flink's Job Server, the expansion service will be
+  started on port 8097. For a different address, please set the
+  expansion_service parameter.
+
+  **More information**
+
+  For more information regarding cross-language transforms see:
+  - https://beam.apache.org/roadmap/portability/
+
+  For more information specific to Flink runner see:
+  - https://beam.apache.org/documentation/runners/flink/
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import time
+from typing import List
+from typing import NamedTuple
+from typing import Optional
+from typing import Tuple
+
+from past.builtins import unicode
+
+from apache_beam import BeamJarExpansionService
+from apache_beam import ExternalTransform
+from apache_beam import NamedTupleBasedPayloadBuilder
+
+__all__ = [
+    'WriteToKinesis',
+    'ReadDataFromKinesis',
+    'InitialPositionInStream',
+    'WatermarkPolicy',
+]
+
+
+def default_io_expansion_service():
+  return BeamJarExpansionService(
+      'sdks:java:io:kinesis:expansion-service:shadowJar')
+
+
+WriteToKinesisSchema = NamedTuple(
+    'WriteToKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('partition_key', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('producer_properties', Optional[List[Tuple[unicode, unicode]]]),
+    ],
+)
+
+
+class WriteToKinesis(ExternalTransform):
+  """
+    An external PTransform which writes byte array stream to Amazon Kinesis.
+
+    Experimental; no backwards compatibility guarantees.
+  """
+  URN = 'beam:external:java:kinesis:write:v1'
+
+  def __init__(
+      self,
+      stream_name,
+      aws_access_key,
+      aws_secret_key,
+      region,
+      partition_key,
+      service_endpoint=None,
+      verify_certificate=None,
+      producer_properties=None,
+      expansion_service=None,
+  ):
+    """
+    Initializes a write operation to Kinesis.
+
+    :param stream_name: Kinesis stream name.
+    :param aws_access_key: Kinesis access key.
+    :param aws_secret_key: Kinesis access key secret.
+    :param region: AWS region. Example: 'us-east-1'.
+    :param service_endpoint: Kinesis service endpoint
+    :param verify_certificate: Enable or disable certificate verification.
+        Never set to False on production. True by default.
+    :param partition_key: Specify default partition key.
+    :param producer_properties: Specify the configuration properties for Kinesis
+        Producer Library (KPL) as List[KV[string, string]].
+        Example: [('CollectionMaxCount', '1000'), ('ConnectTimeout', '10000')]

Review comment:
       Can you make this argument accept a mapping and convert it to `List[Tuple[..]]` as is done in KafkaIO?




----------------------------------------------------------------
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 #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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


   Please resolve the conflict.


----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666382582


   Run Python 3.8 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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r459245205



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisReadTransformRegistrar.java
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.
+ */
+package org.apache.beam.sdk.io.kinesis;
+
+import com.amazonaws.regions.Regions;
+import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream;
+import com.google.auto.service.AutoService;
+import java.util.Map;
+import org.apache.beam.sdk.annotations.Experimental;
+import org.apache.beam.sdk.annotations.Experimental.Kind;
+import org.apache.beam.sdk.expansion.ExternalTransformRegistrar;
+import org.apache.beam.sdk.transforms.ExternalTransformBuilder;
+import org.apache.beam.sdk.transforms.PTransform;
+import org.apache.beam.sdk.values.PBegin;
+import org.apache.beam.sdk.values.PCollection;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableMap;
+import org.joda.time.Duration;
+import org.joda.time.Instant;
+
+/** Exposes {@link KinesisIO.Read} as an external transform for cross-language usage. */
+@Experimental(Kind.PORTABILITY)
+@AutoService(ExternalTransformRegistrar.class)
+public class KinesisReadTransformRegistrar implements ExternalTransformRegistrar {

Review comment:
       I much prefer extracting as much code as possible but since everything is in one package in Kinesis then it makes sense. I'll do this change in Write PR. To be honest I'm not sure whether splitting this to 4PRs is a good idea, but I'll give it a try




----------------------------------------------------------------
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] yuanhunglo commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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


   I tried that and received this error. I'll wait till 2.25.0 release to try again. Thanks!
   
   > _InactiveRpcError: <_InactiveRpcError of RPC that terminated with:
   	status = StatusCode.UNAVAILABLE
   	details = "failed to connect to all addresses"
   	debug_error_string = "{"created":"@1602094791.569834934","description":"Failed to pick subchannel","file":"src/core/ext/filters/client_channel/client_channel.cc","file_line":4133,"referenced_errors":[{"created":"@1602094791.569831898","description":"failed to connect to all addresses","file":"src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc","file_line":397,"grpc_status":14}]}"
   


----------------------------------------------------------------
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] TheNeuralBit commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

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



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisReadTransformRegistrar.java
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.
+ */
+package org.apache.beam.sdk.io.kinesis;
+
+import com.amazonaws.regions.Regions;
+import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream;
+import com.google.auto.service.AutoService;
+import java.util.Map;
+import org.apache.beam.sdk.annotations.Experimental;
+import org.apache.beam.sdk.annotations.Experimental.Kind;
+import org.apache.beam.sdk.expansion.ExternalTransformRegistrar;
+import org.apache.beam.sdk.transforms.ExternalTransformBuilder;
+import org.apache.beam.sdk.transforms.PTransform;
+import org.apache.beam.sdk.values.PBegin;
+import org.apache.beam.sdk.values.PCollection;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableMap;
+import org.joda.time.Duration;
+import org.joda.time.Instant;
+
+/** Exposes {@link KinesisIO.Read} as an external transform for cross-language usage. */
+@Experimental(Kind.PORTABILITY)
+@AutoService(ExternalTransformRegistrar.class)
+public class KinesisReadTransformRegistrar implements ExternalTransformRegistrar {

Review comment:
       Yeah I think I would actually prefer reviewing it as one big PR if that's ok with you




----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683606541


   Run Python 3.8 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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-706002739


   Sorry, I forgot there is some additional setup needed to run it locally.
   
   You also need to build the jar of the runner you are using - example for Flink (you don't need it for DirectRunner)
   ```
   ./gradlew :runners:flink:1.10:job-server:shadowJar
   ```
   Current python container:
   ```
   ./gradlew :sdks:python:container:py37:docker
   ```
   Building java sdk container should not be needed - pulling the latest (2.24) from docker hub should be sufficient. But to be on the safe side:
   ```
   ./gradlew :sdks:java:container:docker
   docker tag apache/beam_java8_sdk:2.26.0.dev apache/beam_java8_sdk:2.24.0.dev 
   ```
   
   It will definitely be the easiest to wait for 2.25 but on the other hand it's a long 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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666363032


   @TheNeuralBit Forget what I just said. I've just found that the expansion-service has an indirect dependency on harness and I managed to set test properties in the gradle file. Now both read and write seem to work locally, I must review why I still have certification problem on jenkins. 
   I don't know why python linter failed on an unrelated file: spark_uber_jar_job_server_test
   
   Edit: I'm an idiot. I was using java container with modified harness with system properties all this time.. So unfortunately the case with read test is still not solved.


----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **decrease** coverage by `0.12%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   - Coverage   40.22%   40.10%   -0.13%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    54025     +356     
   ==========================================
   + Hits        21587    21665      +78     
   - Misses      32082    32360     +278     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   | [.../runners/portability/fn\_api\_runner/translations.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3RyYW5zbGF0aW9ucy5weQ==) | `14.00% <0.00%> (+0.22%)` | :arrow_up: |
   | [...beam/runners/interactive/background\_caching\_job.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9iYWNrZ3JvdW5kX2NhY2hpbmdfam9iLnB5) | `25.92% <0.00%> (+0.68%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666458687


   Run Python 3.8 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] piotr-szuberski commented on pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-662426639


   R: @TheNeuralBit  Could I ask you for 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] [beam] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r474416306



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -295,14 +300,16 @@
   private static final int DEFAULT_NUM_RETRIES = 6;
 
   /** Returns a new {@link Read} transform for reading from Kinesis. */
-  public static Read read() {
-    return new AutoValue_KinesisIO_Read.Builder()
-        .setMaxNumRecords(Long.MAX_VALUE)
-        .setUpToDateThreshold(Duration.ZERO)
-        .setWatermarkPolicyFactory(WatermarkPolicyFactory.withArrivalTimePolicy())
-        .setRateLimitPolicyFactory(RateLimitPolicyFactory.withoutLimiter())
-        .setMaxCapacityPerShard(ShardReadersPool.DEFAULT_CAPACITY_PER_SHARD)
-        .build();
+  public static Read<KinesisRecord> read() {

Review comment:
       The worst thing I can imagine is that a user will get a raw type warning if he used it like `KinesisIO.Read read = KinesisIO.read();`




----------------------------------------------------------------
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] aromanenko-dev commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
aromanenko-dev commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r481276587



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -295,14 +300,16 @@
   private static final int DEFAULT_NUM_RETRIES = 6;
 
   /** Returns a new {@link Read} transform for reading from Kinesis. */
-  public static Read read() {
-    return new AutoValue_KinesisIO_Read.Builder()
-        .setMaxNumRecords(Long.MAX_VALUE)
-        .setUpToDateThreshold(Duration.ZERO)
-        .setWatermarkPolicyFactory(WatermarkPolicyFactory.withArrivalTimePolicy())
-        .setRateLimitPolicyFactory(RateLimitPolicyFactory.withoutLimiter())
-        .setMaxCapacityPerShard(ShardReadersPool.DEFAULT_CAPACITY_PER_SHARD)
-        .build();
+  public static Read<KinesisRecord> read() {

Review comment:
       I think it should be fine if it doesn't require a user code 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.

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



[GitHub] [beam] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **increase** coverage by `0.02%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   + Coverage   40.22%   40.24%   +0.02%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    53722      +53     
   ==========================================
   + Hits        21587    21621      +34     
   - Misses      32082    32101      +19     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   | [...beam/runners/interactive/background\_caching\_job.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9iYWNrZ3JvdW5kX2NhY2hpbmdfam9iLnB5) | `25.92% <0.00%> (+0.68%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-683598608


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=h1) Report
   > Merging [#12297](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/9aa6c39e96ec8005c6a47a24bb6bda26cc0ae1e0?el=desc) will **increase** coverage by `0.02%`.
   > The diff coverage is `66.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12297/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12297      +/-   ##
   ==========================================
   + Coverage   40.22%   40.24%   +0.02%     
   ==========================================
     Files         454      455       +1     
     Lines       53669    53717      +48     
   ==========================================
   + Hits        21587    21619      +32     
   - Misses      32082    32098      +16     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/kinesis.py](https://codecov.io/gh/apache/beam/pull/12297/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8va2luZXNpcy5weQ==) | `66.66% <66.66%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=footer). Last update [9aa6c39...42c62b1](https://codecov.io/gh/apache/beam/pull/12297?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-664541514


   @TheNeuralBit I've moved all commits to this PR. I have a feeling that big PR is better than few smaller PRs dependent of each other - they are much less maintainable. I'm used to Gerrit where each dependent commit is a PR rebased on the previous commit.
   
   I tried to use testcontainers with localstack but I encountered a problem that I can create, read and write streams via boto3, but I'm unable to use it with Beam. What I found is that kinesis java sdk by default uses CBOR which can be disabled by executing java program with kinesis sdk with executing it with -Dcom.amazonaws.sdk.disableCertChecking=true. But I don't have an idea how to do that in beam's execution model - do you have any idea or suggestion how could I do that?
   
   In general the tests before introducing localstack work correctly. Commits after the localstack introduction don't work, I just put vanilla kinesis test to indicate that the container seems to work properly.
   
   Edit: write test is now working after adding disabling certificate verification option.


----------------------------------------------------------------
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] TheNeuralBit commented on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

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






----------------------------------------------------------------
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] piotr-szuberski edited a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski edited a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666363032


   @TheNeuralBit <del>Forget what I just said. I've just found that the expansion-service has an indirect dependency on harness and I managed to set test properties in the gradle file. Now both read and write seem to work locally, I must review why I still have certification problem on jenkins. </del>
   I don't know why python linter failed on an unrelated file: spark_uber_jar_job_server_test
   
   Edit: I'm an idiot. I was using java container with modified harness with system properties all this time.. So unfortunately the case with read test is still not solved.


----------------------------------------------------------------
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] aromanenko-dev commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
aromanenko-dev commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r481276587



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -295,14 +300,16 @@
   private static final int DEFAULT_NUM_RETRIES = 6;
 
   /** Returns a new {@link Read} transform for reading from Kinesis. */
-  public static Read read() {
-    return new AutoValue_KinesisIO_Read.Builder()
-        .setMaxNumRecords(Long.MAX_VALUE)
-        .setUpToDateThreshold(Duration.ZERO)
-        .setWatermarkPolicyFactory(WatermarkPolicyFactory.withArrivalTimePolicy())
-        .setRateLimitPolicyFactory(RateLimitPolicyFactory.withoutLimiter())
-        .setMaxCapacityPerShard(ShardReadersPool.DEFAULT_CAPACITY_PER_SHARD)
-        .build();
+  public static Read<KinesisRecord> read() {

Review comment:
       I think it should be fine if it doesn't require a user code changes.




----------------------------------------------------------------
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] TheNeuralBit commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

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



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -305,6 +306,10 @@ public static Read read() {
         .build();
   }
 
+  public static ReadData readData(Read read) {
+    return new ReadData(read);
+  }
+

Review comment:
       I think this is an atypical API for an IO. If we add something like `readData` to `KinesisIO` (instead of just hiding it in the ExternalTransformBuilder) it would be preferable to make Read generic and have two methods like this:
   
   ```java
   public static Read<KinesisRecord> read() {
     return new Read<KinesisRecord>();
   }
   
   public static Read<byte[]> readData() {
     return new Read<byte[]>((record) -> record.getDataAsBytes());
   }
   ```
   
   That way they can both be configured in the same way:
   ```
   PCollection<KinesisRecord> records = p.apply(KinesisIO.read().withStreamName("streamName").with...)
   PCollection<byte[]> data = p.apply(KinesisIO.readData().withStreamName("streamName").with...)
   ```
   
   This is what PubsubIO does:
   https://github.com/apache/beam/blob/ecedd3e654352f1b51ab2caae0fd4665403bd0eb/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubIO.java#L439-L444
   https://github.com/apache/beam/blob/ecedd3e654352f1b51ab2caae0fd4665403bd0eb/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubIO.java#L464-L469
   




----------------------------------------------------------------
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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r468391276



##########
File path: sdks/python/apache_beam/io/kinesis.py
##########
@@ -0,0 +1,317 @@
+#
+# 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.
+#
+
+"""PTransforms for supporting Kinesis streaming in Python pipelines.
+
+  These transforms are currently supported by Beam Flink and Spark portable
+  runners.
+
+  **Setup**
+
+  Transforms provided in this module are cross-language transforms
+  implemented in the Beam Java SDK. During the pipeline construction, Python SDK
+  will connect to a Java expansion service to expand these transforms.
+  To facilitate this, a small amount of setup is needed before using these
+  transforms in a Beam Python pipeline.
+
+  There are several ways to setup cross-language Kinesis transforms.
+
+  * Option 1: use the default expansion service
+  * Option 2: specify a custom expansion service
+
+  See below for details regarding each of these options.
+
+  *Option 1: Use the default expansion service*
+
+  This is the recommended and easiest setup option for using Python Kinesis
+  transforms. This option is only available for Beam 2.24.0 and later.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Install Java runtime in the computer from where the pipeline is constructed
+    and make sure that 'java' command is available.
+
+  In this option, Python SDK will either download (for released Beam version) or
+  build (when running from a Beam Git clone) a expansion service jar and use
+  that to expand transforms. Currently Kinesis transforms use the
+  'beam-sdks-java-io-kinesis-expansion-service' jar for this purpose.
+
+  *Option 2: specify a custom expansion service*
+
+  In this option, you startup your own expansion service and provide that as
+  a parameter when using the transforms provided in this module.
+
+  This option requires following pre-requisites before running the Beam
+  pipeline.
+
+  * Startup your own expansion service.
+  * Update your pipeline to provide the expansion service address when
+    initiating Kinesis transforms provided in this module.
+
+  Flink Users can use the built-in Expansion Service of the Flink Runner's
+  Job Server. If you start Flink's Job Server, the expansion service will be
+  started on port 8097. For a different address, please set the
+  expansion_service parameter.
+
+  **More information**
+
+  For more information regarding cross-language transforms see:
+  - https://beam.apache.org/roadmap/portability/
+
+  For more information specific to Flink runner see:
+  - https://beam.apache.org/documentation/runners/flink/
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import time
+from typing import List
+from typing import NamedTuple
+from typing import Optional
+from typing import Tuple
+
+from past.builtins import unicode
+
+from apache_beam import BeamJarExpansionService
+from apache_beam import ExternalTransform
+from apache_beam import NamedTupleBasedPayloadBuilder
+
+__all__ = [
+    'WriteToKinesis',
+    'ReadDataFromKinesis',
+    'InitialPositionInStream',
+    'WatermarkPolicy',
+]
+
+
+def default_io_expansion_service():
+  return BeamJarExpansionService(
+      'sdks:java:io:kinesis:expansion-service:shadowJar')
+
+
+WriteToKinesisSchema = NamedTuple(
+    'WriteToKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('partition_key', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('producer_properties', Optional[List[Tuple[unicode, unicode]]]),
+    ],
+)
+
+
+class WriteToKinesis(ExternalTransform):
+  """
+    An external PTransform which writes byte array stream to Amazon Kinesis.
+
+    Experimental; no backwards compatibility guarantees.
+  """
+  URN = 'beam:external:java:kinesis:write:v1'
+
+  def __init__(
+      self,
+      stream_name,
+      aws_access_key,
+      aws_secret_key,
+      region,
+      partition_key,
+      service_endpoint=None,
+      verify_certificate=None,
+      producer_properties=None,
+      expansion_service=None,
+  ):
+    """
+    Initializes a write operation to Kinesis.
+
+    :param stream_name: Kinesis stream name.
+    :param aws_access_key: Kinesis access key.
+    :param aws_secret_key: Kinesis access key secret.
+    :param region: AWS region. Example: 'us-east-1'.
+    :param service_endpoint: Kinesis service endpoint
+    :param verify_certificate: Enable or disable certificate verification.
+        Never set to False on production. True by default.
+    :param partition_key: Specify default partition key.
+    :param producer_properties: Specify the configuration properties for Kinesis
+        Producer Library (KPL) as List[KV[string, string]].
+        Example: [('CollectionMaxCount', '1000'), ('ConnectTimeout', '10000')]
+    :param expansion_service: The address (host:port) of the ExpansionService.
+    """
+    super(WriteToKinesis, self).__init__(
+        self.URN,
+        NamedTupleBasedPayloadBuilder(
+            WriteToKinesisSchema(
+                stream_name=stream_name,
+                aws_access_key=aws_access_key,
+                aws_secret_key=aws_secret_key,
+                region=region,
+                partition_key=partition_key,
+                service_endpoint=service_endpoint,
+                verify_certificate=verify_certificate,
+                producer_properties=producer_properties,
+            )),
+        expansion_service or default_io_expansion_service(),
+    )
+
+
+ReadFromKinesisSchema = NamedTuple(
+    'ReadFromKinesisSchema',
+    [
+        ('stream_name', unicode),
+        ('aws_access_key', unicode),
+        ('aws_secret_key', unicode),
+        ('region', unicode),
+        ('service_endpoint', Optional[unicode]),
+        ('verify_certificate', Optional[bool]),
+        ('max_num_records', Optional[int]),
+        ('max_read_time', Optional[int]),
+        ('initial_position_in_stream', Optional[unicode]),
+        ('initial_timestamp_in_stream', Optional[int]),
+        ('request_records_limit', Optional[int]),
+        ('up_to_date_threshold', Optional[int]),
+        ('max_capacity_per_shard', Optional[int]),
+        ('watermark_policy', Optional[unicode]),
+        ('watermark_idle_duration_threshold', Optional[int]),
+        ('rate_limit', Optional[int]),
+    ],
+)
+
+
+class InitialPositionInStream:
+  LATEST = 'LATEST'
+  TRIM_HORIZON = 'TRIM_HORIZON'
+  AT_TIMESTAMP = 'AT_TIMESTAMP'
+
+
+class WatermarkPolicy:
+  PROCESSING_TYPE = 'PROCESSING_TYPE'
+  ARRIVAL_TIME = 'ARRIVAL_TIME'
+
+
+class ReadDataFromKinesis(ExternalTransform):
+  """
+    An external PTransform which reads byte array stream from Amazon Kinesis.
+
+    Experimental; no backwards compatibility guarantees.
+  """
+  URN = 'beam:external:java:kinesis:read_data:v1'
+
+  def __init__(
+      self,
+      stream_name,
+      aws_access_key,
+      aws_secret_key,
+      region,
+      service_endpoint=None,
+      verify_certificate=None,
+      max_num_records=None,
+      max_read_time=None,
+      initial_position_in_stream=None,
+      initial_timestamp_in_stream=None,
+      request_records_limit=None,
+      up_to_date_threshold=None,
+      max_capacity_per_shard=None,
+      watermark_policy=None,
+      watermark_idle_duration_threshold=None,
+      rate_limit=None,
+      expansion_service=None,
+  ):
+    """
+    Initializes a read operation from Kinesis.
+
+    :param stream_name: Kinesis stream name.
+    :param aws_access_key: Kinesis access key.
+    :param aws_secret_key: Kinesis access key secret.
+    :param region: AWS region. Example: 'us-east-1'.
+    :param service_endpoint: Kinesis service endpoint
+    :param verify_certificate: Enable or disable certificate verification.
+        Never set to False on production. True by default.
+    :param max_num_records: Specifies to read at most a given number of records.
+        Must be greater than 0.
+    :param max_read_time: Specifies to read records during x seconds.
+    :param initial_timestamp_in_stream: Specify reading beginning at the given
+        timestamp in seconds. Must be in the past.
+    :param initial_position_in_stream: Specify reading from some initial
+        position in stream. Possible values:
+        LATEST - Start after the most recent data record (fetch new data).
+        TRIM_HORIZON - Start from the oldest available data record.
+        AT_TIMESTAMP - Start from the record at or after the specified
+        server-side timestamp.
+    :param request_records_limit: Specifies the maximum number of records in
+        GetRecordsResult returned by GetRecords call which is limited by 10K
+        records. If should be adjusted according to average size of data record
+        to prevent shard overloading. More at:
+        docs.aws.amazon.com/kinesis/latest/APIReference/API_GetRecords.html
+    :param up_to_date_threshold: Specifies how late in seconds records consumed
+        by this source can be to still be considered on time. Defaults to zero.
+    :param max_capacity_per_shard: Specifies the maximum number of messages per
+        one shard. Defaults to 10'000.
+    :param watermark_policy: Specifies the watermark policy. Possible values:
+        PROCESSING_TYPE, ARRIVAL_TIME. Defaults to ARRIVAL_TIME.
+    :param watermark_idle_duration_threshold: Use only when watermark policy is
+        ARRIVAL_TIME. Denotes the duration for which the watermark can be idle.
+        Passed in seconds.
+    :param rate_limit: Sets fixed rate policy for given seconds value. By
+        default there is no rate limit.
+    :param expansion_service: The address (host:port) of the ExpansionService.
+    """
+    if watermark_policy:
+      assert watermark_policy == WatermarkPolicy.ARRIVAL_TIME or\
+             watermark_policy == WatermarkPolicy.PROCESSING_TYPE
+
+    if initial_position_in_stream:
+      i = initial_position_in_stream
+      assert i == InitialPositionInStream.AT_TIMESTAMP or \
+             i == InitialPositionInStream.LATEST or \
+             i == InitialPositionInStream.TRIM_HORIZON
+
+    if request_records_limit:
+      assert 0 < request_records_limit <= 10000
+
+    if initial_timestamp_in_stream:
+      assert initial_timestamp_in_stream < time.time()

Review comment:
       I think it's a good idea since kinesis java sdk will verify it later anyway. One thing to discuss is whether I should use milliseconds or just seconds to set the Instants in java. For now I've chosen seconds, but maybe millis are always considered as a better option? WDYT?




----------------------------------------------------------------
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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO.Read external transform

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r459259999



##########
File path: sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -561,6 +569,41 @@ public Read withMaxCapacityPerShard(Integer maxCapacity) {
     }
   }
 
+  /**
+   * A {@link PTransform} to read from Kinesis stream. Similar to {@link KinesisIO.Read}, but
+   * removes Kinesis metatdata and returns a {@link PCollection} of {@link byte[]}. See {@link
+   * KinesisIO} for more information on usage and configuration of reader.
+   */
+  public static class TypedWithoutMetadata extends PTransform<PBegin, PCollection<byte[]>> {

Review comment:
       My first attempt was to register a KnownCoder with coder registrar for KinesisRecord, but later I noticed that datetime is missing so I tried to send longs. But even so I still was getting beam:coders:javasdk:0.1 when I tried to use it in python.




----------------------------------------------------------------
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] piotr-szuberski removed a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski removed a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666350605


   Run Python 3.8 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] piotr-szuberski removed a comment on pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski removed a comment on pull request #12297:
URL: https://github.com/apache/beam/pull/12297#issuecomment-666472634


   Run Python 3.8 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] piotr-szuberski commented on a change in pull request #12297: [BEAM-10137] Add KinesisIO for cross-language usage with python wrapper

Posted by GitBox <gi...@apache.org>.
piotr-szuberski commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r469072126



##########
File path: sdks/python/apache_beam/io/external/xlang_kinesisio_it_test.py
##########
@@ -0,0 +1,308 @@
+#
+# 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 Python cross-language pipelines for Java KinesisIO.
+
+If you want to run the tests on localstack then run it just with pipeline
+options.
+
+To test it on a real AWS account you need to pass some additional params, e.g.:
+python setup.py nosetests \
+--tests=apache_beam.io.external.xlang_kinesisio_it_test \
+--test-pipeline-options="
+  --use_real_aws
+  --aws_kinesis_stream=<STREAM_NAME>
+  --aws_access_key=<AWS_ACCESS_KEY>
+  --aws_secret_key=<AWS_SECRET_KEY>
+  --aws_region=<AWS_REGION>
+  --runner=FlinkRunner"
+"""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import argparse
+import logging
+import time
+import unittest
+import uuid
+
+import apache_beam as beam
+from apache_beam.io.kinesis import InitialPositionInStream
+from apache_beam.io.kinesis import ReadDataFromKinesis
+from apache_beam.io.kinesis import WriteToKinesis
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import StandardOptions
+from apache_beam.testing.test_pipeline import TestPipeline
+from apache_beam.testing.util import assert_that
+from apache_beam.testing.util import equal_to
+
+# pylint: disable=wrong-import-order, wrong-import-position, ungrouped-imports
+try:
+  import boto3
+except ImportError:
+  boto3 = None
+
+try:
+  from testcontainers.core.container import DockerContainer
+except ImportError:
+  DockerContainer = None
+# pylint: enable=wrong-import-order, wrong-import-position, ungrouped-imports
+
+LOCALSTACK_VERSION = '0.11.3'
+NUM_RECORDS = 10
+NOW = time.time()
+RECORD = b'record' + str(uuid.uuid4()).encode()
+
+
+@unittest.skipUnless(DockerContainer, 'testcontainers is not installed.')
+@unittest.skipUnless(boto3, 'boto3 is not installed.')
+@unittest.skipUnless(
+    TestPipeline().get_pipeline_options().view_as(StandardOptions).runner,
+    'Do not run this test on precommit suites.')
+class CrossLanguageKinesisIOTest(unittest.TestCase):
+  @unittest.skipUnless(
+      TestPipeline().get_option('aws_kinesis_stream'),
+      'Cannot test on real aws without pipeline options provided')
+  def test_kinesis_io_roundtrip(self):
+    # TODO: enable this test for localstack once BEAM-10664 is resolved
+    self.run_kinesis_write()
+    self.run_kinesis_read()
+
+  @unittest.skipIf(
+      TestPipeline().get_option('aws_kinesis_stream'),
+      'Do not test on localstack when pipeline options were provided')
+  def test_kinesis_write(self):
+    # TODO: remove this test once BEAM-10664 is resolved
+    self.run_kinesis_write()
+    records = self.kinesis_helper.read_from_stream(self.aws_kinesis_stream)
+    self.assertEqual(
+        sorted(records),
+        sorted([RECORD + str(i).encode() for i in range(NUM_RECORDS)]))
+
+  def run_kinesis_write(self):
+    with TestPipeline(options=PipelineOptions(self.pipeline_args)) as p:
+      p.not_use_test_runner_api = True
+      _ = (
+          p
+          | 'Impulse' >> beam.Impulse()
+          | 'Generate' >> beam.FlatMap(lambda x: range(NUM_RECORDS))  # pylint: disable=range-builtin-not-iterating
+          | 'Map to bytes' >>
+          beam.Map(lambda x: RECORD + str(x).encode()).with_output_types(bytes)
+          | 'WriteToKinesis' >> WriteToKinesis(
+              stream_name=self.aws_kinesis_stream,
+              aws_access_key=self.aws_access_key,
+              aws_secret_key=self.aws_secret_key,
+              region=self.aws_region,
+              service_endpoint=self.aws_service_endpoint,
+              verify_certificate=(not self.use_localstack),
+              partition_key='1',
+          ))
+
+  def run_kinesis_read(self):
+    records = [RECORD + str(i).encode() for i in range(NUM_RECORDS)]
+
+    with TestPipeline(options=PipelineOptions(self.pipeline_args)) as p:
+      result = (
+          p
+          | 'ReadFromKinesis' >> ReadDataFromKinesis(
+              stream_name=self.aws_kinesis_stream,
+              aws_access_key=self.aws_access_key,
+              aws_secret_key=self.aws_secret_key,
+              region=self.aws_region,
+              service_endpoint=self.aws_service_endpoint,
+              verify_certificate=not self.use_localstack,
+              max_num_records=NUM_RECORDS,
+              max_read_time=300,  # 5min
+              initial_position_in_stream=InitialPositionInStream.AT_TIMESTAMP,
+              initial_timestamp_in_stream=int(NOW),
+          ).with_output_types(bytes))
+      assert_that(result, equal_to(records))
+
+  def set_localstack(self):
+    self.localstack = DockerContainer('localstack/localstack:{}'
+                                      .format(LOCALSTACK_VERSION))\
+      .with_env('SERVICES', 'kinesis')\
+      .with_env('KINESIS_PORT', '4568')\
+      .with_env('USE_SSL', 'true')\
+      .with_exposed_ports(4568)\
+      .with_volume_mapping('/var/run/docker.sock', '/var/run/docker.sock', 'rw')
+
+    # Repeat if ReadTimeout is raised.
+    for i in range(4):
+      try:
+        self.localstack.start()
+        break
+      except Exception as e:  # pylint: disable=bare-except
+        if i == 3:
+          logging.error('Could not initialize localstack container')
+          raise e
+
+    self.aws_service_endpoint = 'https://{}:{}'.format(
+        self.localstack.get_container_host_ip(),
+        self.localstack.get_exposed_port('4568'),
+    )
+
+  def setUp(self):
+    parser = argparse.ArgumentParser()
+
+    parser.add_argument(
+        '--aws_kinesis_stream',
+        default='beam_kinesis_xlang',
+        help='Kinesis stream name',
+    )
+    parser.add_argument(
+        '--aws_access_key',
+        default='accesskey',
+        help=('Aws access key'),
+    )
+    parser.add_argument(
+        '--aws_secret_key',
+        default='secretkey',
+        help='Aws secret key',
+    )
+    parser.add_argument(
+        '--aws_region',
+        default='us-east-1',
+        help='Aws region',
+    )
+    parser.add_argument(
+        '--aws_service_endpoint',
+        default=None,
+        help='Url to external aws endpoint',
+    )
+    parser.add_argument(
+        '--use_real_aws',
+        default=False,
+        dest='use_real_aws',
+        action='store_true',
+        help='Flag whether to use real aws for the tests purpose',
+    )
+    parser.add_argument(
+        '--expansion_service',
+        help='Url to externally launched expansion service.',
+    )
+
+    pipeline = TestPipeline()
+    argv = pipeline.get_full_options_as_args()
+
+    known_args, self.pipeline_args = parser.parse_known_args(argv)
+
+    self.aws_kinesis_stream = known_args.aws_kinesis_stream
+    self.aws_access_key = known_args.aws_access_key
+    self.aws_secret_key = known_args.aws_secret_key
+    self.aws_region = known_args.aws_region
+    self.aws_service_endpoint = known_args.aws_service_endpoint
+    self.use_localstack = not known_args.use_real_aws
+    self.expansion_service = known_args.expansion_service
+
+    if self.use_localstack:
+      self.set_localstack()
+
+    self.kinesis_helper = KinesisHelper(
+        self.aws_access_key,
+        self.aws_secret_key,
+        self.aws_region,
+        self.aws_service_endpoint.replace('https', 'http')

Review comment:
       Yes, otherwise test fails on certificate verification. Http/Https is the way the python sdk chooses whether to verify or not.




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