You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by yssharma <gi...@git.apache.org> on 2017/05/18 12:53:42 UTC

[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

GitHub user yssharma opened a pull request:

    https://github.com/apache/spark/pull/18029

    [SPARK-20168][WIP][DStream] Add changes to use kinesis fetches from specified timestamp

    ## What changes were proposed in this pull request?
    
    Kinesis client can resume from a specified timestamp while creating a stream. We should have option to pass a timestamp in config to allow kinesis to resume from the given timestamp.
    
    The patch introduces a new `KinesisInitialPositionInStream` that takes the `InitialPositionInStream` with the `timestamp` information that can be used to resume kinesis fetches from the provided timestamp.
    
    ## How was this patch tested?
    
    todo
    
    cc : @budde @brkyvz 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/yssharma/spark ysharma/kcl_resume

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/18029.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #18029
    
----

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134067474
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisReceiver.scala ---
    @@ -148,18 +149,28 @@ private[kinesis] class KinesisReceiver[T](
     
         kinesisCheckpointer = new KinesisCheckpointer(receiver, checkpointInterval, workerId)
         val kinesisProvider = kinesisCreds.provider
    -    val kinesisClientLibConfiguration = new KinesisClientLibConfiguration(
    +    var kinesisClientLibConfiguration = new KinesisClientLibConfiguration(
    --- End diff --
    
    Keep this a val, but you can introduce a new scope with a temp val using braces, e.g.: 
    
    ```scala
    val kinesisClientLibConfiguration = {
      val baseClientLibConfiguration = new KinesisClientLibConfiguration(
          checkpointAppName,
          streamName,
          ...
        .withKinesisEndpoint(endpointUrl)
        .withInitialPositionInStream(initialPosition.initialPositionInStream)
        ...
    
      initialPosition match {
        // see comment below
        ...
      }
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134063427
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,107 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  var initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  def instance: InitialPosition = this
    --- End diff --
    
    Is this necessary?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Thanks for the comments @brkyvz . I will be working on the changes and update the PR very soon.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r136725034
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest: InitialPosition = Latest
    --- End diff --
    
    nit: The first letters can be capitalized. `Latest`, `TrimHorizon`, `AtTimestamp`... Similar to `Trigger.Once()` or `Trigger.ProcessingTime(..)` in Structured Streaming


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82631/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r119724691
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -38,6 +40,7 @@ private[kinesis] class KinesisInputDStream[T: ClassTag](
         val endpointUrl: String,
         val regionName: String,
         val initialPositionInStream: InitialPositionInStream,
    +    val initialPositionInStreamTimestamp: Date,
    --- End diff --
    
    I think there needs to be a better abstraction around the ```initialPositionInStream``` and ```initialPositionInStreamTimestamp``` options. Providing both is redundant as a user would want to specify one or the other. Additionally, if ```initialPositionInStreamTimestamp``` is an optional value then its type should at the very least be ```Option[Date]``` with a default value of ```None```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #85171 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85171/testReport)** for PR 18029 at commit [`e18fdaa`](https://github.com/apache/spark/commit/e18fdaa2b9c70f58b57fc564c137a2dce51d2b25).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r120181290
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -193,6 +197,21 @@ object KinesisInputDStream {
         }
     
         /**
    +     * Sets the Kinesis initial position data to the provided timestamp.
    +     * Sets InitialPositionInStream to [[InitialPositionInStream.AT_TIMESTAMP]]
    +     * and the timestamp to the provided value.
    +     *
    +     * @param timestamp Timestamp to resume the Kinesis stream from a provided
    +     *                  timestamp.
    +     * @return Reference to this [[KinesisInputDStream.Builder]]
    +     */
    +    def withTimestampAtInitialPositionInStream(timestamp: Date) : Builder = {
    --- End diff --
    
    `withInitialPositionAtTimestamp`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137684968
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    --- End diff --
    
    I've implemented the functions with this Capital naming, but still feel a bit salty about this :)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137085859
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest: InitialPosition = Latest
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.TRIM_HORIZON.
    +   * @return [[TrimHorizon]]
    +   */
    +  val trimHorizon: InitialPosition = TrimHorizon
    +
    +  /**
    +   * Returns instance of AtTimestamp with InitialPositionInStream.AT_TIMESTAMP.
    +   * @return [[AtTimestamp]]
    +   */
    +  def atTimestamp(timestamp: Date): InitialPosition = AtTimestamp(timestamp)
    +
    +  /**
    +   * Returns instance of [[InitialPosition]] based on the passed [[InitialPositionInStream]].
    +   * This method is used in KinesisUtils for translating the InitialPositionInStream
    +   * to InitialPosition. This function would be removed when we deprecate the KinesisUtils.
    +   *
    +   * @return [[InitialPosition]]
    +   */
    +  def kinesisInitialPositionInStream(
    --- End diff --
    
    how about `fromKinesisInitialPosition`?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134068186
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,107 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  var initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  def instance: InitialPosition = this
    --- End diff --
    
    Looks like it is for Java compatibility


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82626/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #85171 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85171/testReport)** for PR 18029 at commit [`e18fdaa`](https://github.com/apache/spark/commit/e18fdaa2b9c70f58b57fc564c137a2dce51d2b25).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class KinesisInitialPositions `
      * `    public static class Latest implements KinesisInitialPosition `
      * `    public static class TrimHorizon implements KinesisInitialPosition `
      * `    public static class AtTimestamp implements KinesisInitialPosition `


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137926490
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest: InitialPosition = Latest
    --- End diff --
    
    Implemented new java wrapper for the Api !


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82625/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Thanks for being so helpful and patient on this one @brkyvz . I will leave this with you now for your final ☑️ if you're happy with it :)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134120489
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest : InitialPosition = Latest
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.TRIM_HORIZON.
    +   * @return [[TrimHorizon]]
    +   */
    +  val trimHorizon : InitialPosition = TrimHorizon
    +
    +  /**
    +   * Returns instance of AtTimestamp with InitialPositionInStream.AT_TIMESTAMP.
    +   * @return [[AtTimestamp]]
    +   */
    +  def atTimestamp(timestamp: Date) : InitialPosition = AtTimestamp(timestamp)
    +
    +  /**
    +   * Returns instance of [[InitialPosition]] based on the passed [[InitialPositionInStream]].
    +   * This method is used in KinesisUtils for translating the InitialPositionInStream
    +   * to InitialPosition. This function would be removed when we deprecate the KinesisUtils.
    +   *
    +   * @return [[InitialPosition]]
    +   */
    +  def kinesisInitialPositionInStream(
    +    initialPositionInStream: InitialPositionInStream) : InitialPosition = {
    --- End diff --
    
    *nit* Indent this one more level. Remove space before ```:```.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77043 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77043/testReport)** for PR 18029 at commit [`7cf935b`](https://github.com/apache/spark/commit/7cf935bfd1d0c12f4a9c9477f2af1e68457559e9).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82635 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82635/testReport)** for PR 18029 at commit [`72703a0`](https://github.com/apache/spark/commit/72703a072e34407b52d555bab98a435414d2ed25).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #80876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80876/testReport)** for PR 18029 at commit [`7d9a08a`](https://github.com/apache/spark/commit/7d9a08aa4471e2b76bb15e72325ef717ab469ee2).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Added review suggestions @budde !


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82624 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82624/testReport)** for PR 18029 at commit [`05cad04`](https://github.com/apache/spark/commit/05cad04d742811689f2f5da7685244bf5c5937a5).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #80871 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80871/testReport)** for PR 18029 at commit [`c3622b9`](https://github.com/apache/spark/commit/c3622b91332775a363c68d9571f4f5dbc3af84fc).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77121 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77121/testReport)** for PR 18029 at commit [`b71a8d6`](https://github.com/apache/spark/commit/b71a8d621ff048958dd5f10ef16cf5989026ed5f).
     * This patch **fails from timeout after a configured wait of \`250m\`**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class JavaChiSquareTestExample `
      * `public class JavaCorrelationExample `
      * `case class Cot(child: Expression)`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #79383 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79383/testReport)** for PR 18029 at commit [`2e718f6`](https://github.com/apache/spark/commit/2e718f640f17076f3373c83beead24e503fb4f9a).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #85168 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85168/testReport)** for PR 18029 at commit [`9abf92b`](https://github.com/apache/spark/commit/9abf92b1fe871128c607089faf07c6ef807321e2).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class KinesisInitialPositions `
      * `    public static class Latest implements KinesisInitialPosition `
      * `    public static class TrimHorizon implements KinesisInitialPosition `
      * `    public static class AtTimestamp implements KinesisInitialPosition `


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r143850792
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisReceiver.scala ---
    @@ -148,18 +149,30 @@ private[kinesis] class KinesisReceiver[T](
     
         kinesisCheckpointer = new KinesisCheckpointer(receiver, checkpointInterval, workerId)
         val kinesisProvider = kinesisCreds.provider
    -    val kinesisClientLibConfiguration = new KinesisClientLibConfiguration(
    -          checkpointAppName,
    -          streamName,
    -          kinesisProvider,
    -          dynamoDBCreds.map(_.provider).getOrElse(kinesisProvider),
    -          cloudWatchCreds.map(_.provider).getOrElse(kinesisProvider),
    -          workerId)
    +
    +    val kinesisClientLibConfiguration = {
    +      val baseClientLibConfiguration = new KinesisClientLibConfiguration(
    +        checkpointAppName,
    +        streamName,
    +        kinesisProvider,
    +        dynamoDBCreds.map(_.provider).getOrElse(kinesisProvider),
    +        cloudWatchCreds.map(_.provider).getOrElse(kinesisProvider),
    +        workerId)
             .withKinesisEndpoint(endpointUrl)
    -        .withInitialPositionInStream(initialPositionInStream)
    +        .withInitialPositionInStream(initialPosition.initialPositionInStream)
             .withTaskBackoffTimeMillis(500)
             .withRegionName(regionName)
     
    +      // Update the Kinesis client lib config with timestamp
    +      // if InitialPositionInStream.AT_TIMESTAMP is passed
    +      initialPosition match {
    +        case atTimestamp: AtTimestamp =>
    --- End diff --
    
    nit: 
    ```scala
    initialPosition match {
      case AtTimestamp(ts) =>
        baseClientLibConfiguration.withTimestampAtInitialPositionInStream(ts)
    ...
    }
    
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r158627744
  
    --- Diff: external/kinesis-asl/src/main/java/org/apache/spark/streaming/kinesis/KinesisInitialPositions.java ---
    @@ -0,0 +1,91 @@
    +/*
    + * 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.spark.streaming.kinesis;
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream;
    +
    +import java.io.Serializable;
    +import java.util.Date;
    +
    +/**
    + * A java wrapper for exposing [[InitialPositionInStream]]
    + * to the corresponding Kinesis readers.
    + */
    +interface KinesisInitialPosition {
    +    InitialPositionInStream getPosition();
    +}
    +
    +public class KinesisInitialPositions {
    +    public static class Latest implements KinesisInitialPosition, Serializable {
    +        public Latest() {}
    +
    +        @Override
    +        public InitialPositionInStream getPosition() {
    +            return InitialPositionInStream.LATEST;
    +        }
    +    }
    +
    +    public static class TrimHorizon implements KinesisInitialPosition, Serializable {
    +        public TrimHorizon() {}
    +
    +        @Override
    +        public InitialPositionInStream getPosition() {
    +            return InitialPositionInStream.TRIM_HORIZON;
    +        }
    +    }
    +
    +    public static class AtTimestamp implements KinesisInitialPosition, Serializable {
    +        private Date timestamp;
    +
    +        public AtTimestamp(Date timestamp) {
    +            this.timestamp = timestamp;
    +        }
    +
    +        @Override
    +        public InitialPositionInStream getPosition() {
    +            return InitialPositionInStream.AT_TIMESTAMP;
    +        }
    +
    +        public Date getTimestamp() {
    +            return timestamp;
    +        }
    +    }
    +
    +
    +    /**
    +     * Returns instance of [[KinesisInitialPosition]] based on the passed [[InitialPositionInStream]].
    +     * This method is used in KinesisUtils for translating the InitialPositionInStream
    +     * to InitialPosition. This function would be removed when we deprecate the KinesisUtils.
    +     *
    +     * @return [[InitialPosition]]
    +     */
    +    public static KinesisInitialPosition fromKinesisInitialPosition(
    +            InitialPositionInStream initialPositionInStream) throws UnsupportedOperationException {
    +        if (initialPositionInStream == InitialPositionInStream.LATEST) {
    +            return new Latest();
    +        } else if (initialPositionInStream == InitialPositionInStream.TRIM_HORIZON) {
    +            return new TrimHorizon();
    +        } else {
    +            // InitialPositionInStream.AT_TIMESTAMP is not supported.
    +            // Use InitialPosition.atTimestamp(timestamp) instead.
    +            throw new UnsupportedOperationException(
    +                    "Only InitialPositionInStream.LATEST and InitialPositionInStream.TRIM_HORIZON " +
    +                            "supported in initialPositionInStream(). Please use the initialPosition() from " +
    +                            "builder API in KinesisInputDStream for using InitialPositionInStream.AT_TIMESTAMP");
    +        }
    +    }
    +}
    --- End diff --
    
    nit: new line


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #85168 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85168/testReport)** for PR 18029 at commit [`9abf92b`](https://github.com/apache/spark/commit/9abf92b1fe871128c607089faf07c6ef807321e2).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134068101
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,107 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  var initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * Returns instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  def latest() : InitialPosition = {
    +    Latest
    +  }
    +
    +  /**
    +   * Returns instance of Latest with InitialPositionInStream.TRIM_HORIZON.
    +   * @return [[TrimHorizon]]
    +   */
    +  def trimHorizon() : InitialPosition = {
    +    TrimHorizon
    +  }
    +
    +  /**
    +   * Returns instance of AtTimestamp with InitialPositionInStream.AT_TIMESTAMP.
    +   * @return [[AtTimestamp]]
    +   */
    +  def atTimestamp(timestamp: Date) : InitialPosition = {
    +    AtTimestamp(timestamp)
    +  }
    +
    +  /**
    +   * Returns instance of [[InitialPosition]] based on the passed [[InitialPositionInStream]].
    +   * @return [[InitialPosition]]
    +   */
    +  def kinesisInitialPositionInStream(
    --- End diff --
    
    Ok, I see that it is being used to maintain the original APIs present in ```KinesisUtils```. However, we should be deprecating ```KinesisUtils``` at some (undetermined?) point. Would be good to remove this method at that time as well.
    
    I'd suggest adding a comment to the docs for this method indicating it exists to maintain compatibility with the original ```KinesisUtils``` API.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yashs360 <gi...@git.apache.org>.
Github user yashs360 commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Thanks for your help and patience on this one @brkyvz :)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r143849442
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,101 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest: InitialPosition = Latest
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.TRIM_HORIZON.
    +   * @return [[TrimHorizon]]
    +   */
    +  val trimHorizon: InitialPosition = TrimHorizon
    --- End diff --
    
    We don't need both the Scala API for this anymore. We can just use the Java objects directly.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77727 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77727/testReport)** for PR 18029 at commit [`4ba5424`](https://github.com/apache/spark/commit/4ba5424184fd4cf4c6e2aed58ae5352806f549fd).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r120236059
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -100,6 +103,7 @@ object KinesisInputDStream {
         private var endpointUrl: Option[String] = None
         private var regionName: Option[String] = None
         private var initialPositionInStream: Option[InitialPositionInStream] = None
    +    private var initialPositionInStreamTimestamp: Option[Date] = None
    --- End diff --
    
    Ah alright, so you're asking to get another `initialPositionInStreamTimestamp`. Thats similar to the `withInitialPositionAtTimestamp`.  Can rename that to suit this purpose.
    
    Another question, The InitialPosition gets passed to the KinesisReceiver. I was passing a timestamp along with the Initial position at the moment. Are we planning to pass the `KinesisClientLibConfiguration` to the `KinesisReceiver` now ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #79384 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79384/testReport)** for PR 18029 at commit [`3ce6870`](https://github.com/apache/spark/commit/3ce6870aecf0ca80734a485ffe2e867bcaf62fc6).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77701/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r119986280
  
    --- Diff: external/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisInputDStreamBuilderSuite.scala ---
    @@ -111,5 +110,28 @@ class KinesisInputDStreamBuilderSuite extends TestSuiteBase with BeforeAndAfterE
         assert(dstream.kinesisCreds == customKinesisCreds)
         assert(dstream.dynamoDBCreds == Option(customDynamoDBCreds))
         assert(dstream.cloudWatchCreds == Option(customCloudWatchCreds))
    +
    +    val yesterday = DateUtils.addDays(new Date, -1)
    +    val dStreamFromTimestamp = builder
    +    .endpointUrl(customEndpointUrl)
    +    .regionName(customRegion)
    +    .initialPositionInStream(InitialPositionInStream.AT_TIMESTAMP, Some(yesterday))
    --- End diff --
    
    @budde Added optional timestamp for resume, but passing as Some() doesn't seem very interesting. Passing a  date directly seems more intuitive. Thoughts ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Thanks for the review @brkyvz . Updated the pull request with version 2.3.0.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r119986035
  
    --- Diff: external/kinesis-asl/src/test/java/org/apache/spark/streaming/kinesis/JavaKinesisInputDStreamBuilderSuite.java ---
    @@ -45,7 +46,7 @@ public void testJavaKinesisDStreamBuilder() {
           .streamName(streamName)
           .endpointUrl(endpointUrl)
           .regionName(region)
    -      .initialPositionInStream(initialPosition)
    +      .initialPositionInStream(initialPosition, scala.Option.apply(null))
    --- End diff --
    
    @budde not having the overloaded methods introduces this backward compatibility issue which I didn't like much. What are your thoughts on this ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134067110
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisReceiver.scala ---
    @@ -148,18 +149,28 @@ private[kinesis] class KinesisReceiver[T](
     
         kinesisCheckpointer = new KinesisCheckpointer(receiver, checkpointInterval, workerId)
         val kinesisProvider = kinesisCreds.provider
    -    val kinesisClientLibConfiguration = new KinesisClientLibConfiguration(
    +    var kinesisClientLibConfiguration = new KinesisClientLibConfiguration(
               checkpointAppName,
               streamName,
               kinesisProvider,
               dynamoDBCreds.map(_.provider).getOrElse(kinesisProvider),
               cloudWatchCreds.map(_.provider).getOrElse(kinesisProvider),
               workerId)
             .withKinesisEndpoint(endpointUrl)
    -        .withInitialPositionInStream(initialPositionInStream)
    +        .withInitialPositionInStream(initialPosition.initialPositionInStream)
             .withTaskBackoffTimeMillis(500)
             .withRegionName(regionName)
     
    +    // Update the Kinesis client lib config with timestamp
    +    // if InitialPositionInStream.AT_TIMESTAMP is passed
    +    kinesisClientLibConfiguration =
    +      if (initialPosition.initialPositionInStream == InitialPositionInStream.AT_TIMESTAMP) {
    --- End diff --
    
    Here's a more-stylish way of doing this in Scala:
    
    ```
    initialPosition match {
      case atTimestamp: AtTimestamp =>
        baseClientLibConfiguration.withTimestampAtInitialPositionInStream(atTimestamp.timestamp)
      case _ =>
        baseClientLibConfiguration
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @yssharma I'll try to take a look later today or tomorrow


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134067693
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,107 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  var initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * Returns instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  def latest() : InitialPosition = {
    +    Latest
    +  }
    +
    +  /**
    +   * Returns instance of Latest with InitialPositionInStream.TRIM_HORIZON.
    +   * @return [[TrimHorizon]]
    +   */
    +  def trimHorizon() : InitialPosition = {
    +    TrimHorizon
    +  }
    +
    +  /**
    +   * Returns instance of AtTimestamp with InitialPositionInStream.AT_TIMESTAMP.
    +   * @return [[AtTimestamp]]
    +   */
    +  def atTimestamp(timestamp: Date) : InitialPosition = {
    +    AtTimestamp(timestamp)
    +  }
    +
    +  /**
    +   * Returns instance of [[InitialPosition]] based on the passed [[InitialPositionInStream]].
    +   * @return [[InitialPosition]]
    +   */
    +  def kinesisInitialPositionInStream(
    --- End diff --
    
    Is this method really necessary? Especially if it can only be used for a subset of the official ```InitialPositionInStream``` implementations?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80876/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #79383 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79383/testReport)** for PR 18029 at commit [`2e718f6`](https://github.com/apache/spark/commit/2e718f640f17076f3373c83beead24e503fb4f9a).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77739 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77739/testReport)** for PR 18029 at commit [`7caa9c5`](https://github.com/apache/spark/commit/7caa9c5b043ce0767cc308caf5d589d9e45494cf).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r119725220
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisUtils.scala ---
    @@ -73,7 +73,7 @@ object KinesisUtils {
         // Setting scope to override receiver stream's scope of "receiver stream"
         ssc.withNamedScope("kinesis stream") {
           new KinesisInputDStream[T](ssc, streamName, endpointUrl, validateRegion(regionName),
    -        initialPositionInStream, kinesisAppName, checkpointInterval, storageLevel,
    +        initialPositionInStream, null, kinesisAppName, checkpointInterval, storageLevel,
    --- End diff --
    
    Passing a ```null``` for an optional value is very un-idiomatic for Scala code. As noted in my previous comment, we should find a way to pass a single config option that represents all possible InitialPositionInStream configurations or at the very least make this argument an ```Option[Date]``` with a default value of ```None```. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @budde @brkyvz could you suggest if the current patch seems ok, or I should make something similar to the case class/ trait ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77121 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77121/testReport)** for PR 18029 at commit [`b71a8d6`](https://github.com/apache/spark/commit/b71a8d621ff048958dd5f10ef16cf5989026ed5f).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82635/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r143850590
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -182,14 +181,14 @@ object KinesisInputDStream {
     
         /**
          * Sets the initial position data is read from in the Kinesis stream. Defaults to
    -     * [[InitialPositionInStream.LATEST]] if no custom value is specified.
    +     * [[InitialPosition.latest]] if no custom value is specified.
    --- End diff --
    
    don't change docs


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77701 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77701/testReport)** for PR 18029 at commit [`f1ffcbd`](https://github.com/apache/spark/commit/f1ffcbd897f5a9f6408458c55dd2b7b63ae73b93).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137245127
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    --- End diff --
    
    Looks good.
    How do you compare the syntax: 
    `InitialPosition initialPosition = TrimHorizon.instance();`
    or, introducing a new java class KinesisInitialPosition.java for:
    `InitialPosition initialPosition = KinesisInitialPosition.TrimHorizon();`


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82631 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82631/testReport)** for PR 18029 at commit [`ed9cab4`](https://github.com/apache/spark/commit/ed9cab48c61627f1b4e1a6f1afc16d96ae24207e).
     * This patch **fails to build**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yashs360 <gi...@git.apache.org>.
Github user yashs360 commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Hi @brkyvz , I've added the new changes with the java classes. Had to make the classes serializable for passing them to the KinesisReceiver. Please have a look when you get time. Thanks.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Making them as singletons is unnecessary. How about this:
    
    ```java
    public interface InitialPosition {
        public InitialPositionInStream toKinesis();
    }
    
    public class InitialPositions {
        public static class Latest implements InitialPosition {
            public Latest() {}
            
            @Override
            public InitialPositionInStream toKinesis() {
                return InitialPositionInStream.LATEST;
            }
        }
    
        public static class TrimHorizon implements InitialPosition {
            public TrimHorizon() {}
            
            @Override
            public InitialPositionInStream toKinesis() {
                return InitialPositionInStream.TRIM_HORIZON;
            }
        }
    
        public static class AtTimestamp implements InitialPosition {
            private Date timestamp;
            
            public AtTimestamp(Date timestamp) {
                this.timestamp = timestamp;
            }
            
            @Override
            public InitialPositionInStream toKinesis() {
                return InitialPositionInStream.AT_TIMESTAMP;
            }
    
            public Date getTimestamp() {
                return timestamp;
            }
        }
    }
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r119724818
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisReceiver.scala ---
    @@ -84,6 +84,7 @@ private[kinesis] class KinesisReceiver[T](
         endpointUrl: String,
         regionName: String,
         initialPositionInStream: InitialPositionInStream,
    +    initialPositionInStreamTimestamp: Date,
    --- End diff --
    
    See my previous comment in **KinesisInputDStream.scala** about these parameters


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r143850857
  
    --- Diff: external/kinesis-asl/src/test/java/org/apache/spark/streaming/kinesis/JavaKinesisInputDStreamBuilderSuite.java ---
    @@ -45,15 +43,15 @@ public void testJavaKinesisDStreamBuilder() {
           .streamName(streamName)
           .endpointUrl(endpointUrl)
           .regionName(region)
    -      .initialPositionInStream(initialPosition)
    --- End diff --
    
    ditto on API change


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r144336164
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -182,14 +182,29 @@ object KinesisInputDStream {
     
         /**
          * Sets the initial position data is read from in the Kinesis stream. Defaults to
    +     * [[Latest]] if no custom value is specified.
    +     *
    +     * @param initialPosition [[InitialPosition]] value specifying where Spark Streaming
    +     *                        will start reading records in the Kinesis stream from
    +     * @return Reference to this [[KinesisInputDStream.Builder]]
    +     */
    +    def initialPosition(initialPosition: InitialPosition): Builder = {
    +      this.initialPosition = Option(initialPosition)
    +      this
    +    }
    +
    +    /**
    +     * Sets the initial position data is read from in the Kinesis stream. Defaults to
          * [[InitialPositionInStream.LATEST]] if no custom value is specified.
    +     * This function would be removed when we deprecate the KinesisUtils.
          *
          * @param initialPosition InitialPositionInStream value specifying where Spark Streaming
          *                        will start reading records in the Kinesis stream from
          * @return Reference to this [[KinesisInputDStream.Builder]]
          */
    +    @deprecated("use initialPosition(initialPosition: InitialPosition)", "2.0.0")
    --- End diff --
    
    2.3.0


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @brkyvz , @budde, @HyukjinKwon  could you please review this sometime. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77775 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77775/testReport)** for PR 18029 at commit [`c4ed442`](https://github.com/apache/spark/commit/c4ed442eb20f56cb346cfb42234f2652ab1883e3).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79382/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85373/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134120477
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest : InitialPosition = Latest
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.TRIM_HORIZON.
    +   * @return [[TrimHorizon]]
    +   */
    +  val trimHorizon : InitialPosition = TrimHorizon
    +
    +  /**
    +   * Returns instance of AtTimestamp with InitialPositionInStream.AT_TIMESTAMP.
    +   * @return [[AtTimestamp]]
    +   */
    +  def atTimestamp(timestamp: Date) : InitialPosition = AtTimestamp(timestamp)
    --- End diff --
    
    *nit* remove space before ```:```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #85181 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85181/testReport)** for PR 18029 at commit [`3c16c47`](https://github.com/apache/spark/commit/3c16c478257c8aed61b1cef4d75360b8bb8b166d).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134120545
  
    --- Diff: external/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisInputDStreamBuilderSuite.scala ---
    @@ -104,12 +103,61 @@ class KinesisInputDStreamBuilderSuite extends TestSuiteBase with BeforeAndAfterE
           .build()
         assert(dstream.endpointUrl == customEndpointUrl)
         assert(dstream.regionName == customRegion)
    -    assert(dstream.initialPositionInStream == customInitialPosition)
    +    assert(dstream.initialPosition.initialPositionInStream
    --- End diff --
    
    *nit* Again, I think this could be simplified to:
    
    ```scala
    assert(dstream.initialPosition == customInitialPosition)
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82624/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82625 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82625/testReport)** for PR 18029 at commit [`06e3969`](https://github.com/apache/spark/commit/06e3969d0d4e19fb2fb989973a7836ec80f27d82).
     * This patch **fails to build**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82623 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82623/testReport)** for PR 18029 at commit [`107c1ab`](https://github.com/apache/spark/commit/107c1aba8b029cac933ab726e45e9f13fabcfccb).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @budde could you please do one last review of this one.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77136 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77136/testReport)** for PR 18029 at commit [`ff9a586`](https://github.com/apache/spark/commit/ff9a58669853ae0508d3ef599947d15a92e1f712).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class KinesisInitialPositionInStream (`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83144/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137083713
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    --- End diff --
    
    why do you need the `instance`s?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #79384 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79384/testReport)** for PR 18029 at commit [`3ce6870`](https://github.com/apache/spark/commit/3ce6870aecf0ca80734a485ffe2e867bcaf62fc6).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82708 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82708/testReport)** for PR 18029 at commit [`ac0ae53`](https://github.com/apache/spark/commit/ac0ae53935510221ebb6a44f6ec30b68fca34343).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81530/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85181/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    
    ```
    ./bin/spark-shell --jars /Users/ysharma/.m2/repository/org/apache/spark/spark-streaming-kinesis-asl_2.11/2.3.0-SNAPSHOT/spark-streaming-kinesis-asl_2.11-2.3.0-SNAPSHOT.jar --packages com.amazonaws:amazon-kinesis-client:1.0.0 -Ylog-classpath
    
    
    import org.apache.spark.SparkConf
    import org.apache.spark.storage.StorageLevel
    import org.apache.spark.streaming.kinesis.KinesisInputDStream
    import org.apache.spark.streaming.{Seconds, StreamingContext}
    import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    
    
    val streamingContext = new StreamingContext(sc, Seconds(30))
    val kinesisStream = KinesisInputDStream.builder
        .streamingContext(streamingContext)
        .endpointUrl("https://kinesis.us-east-1.amazonaws.com")
        .regionName("us-east-1")
        .streamName("a-very-nice-kinesis-app")
        .initialPositionInStream(InitialPositionInStream.TRIM_HORIZON)
        .checkpointAppName("a-very-nice-kinesis-app")
        .checkpointInterval(Seconds(30))
        .storageLevel(StorageLevel.MEMORY_AND_DISK_2)
        .build()
    
    Error:
    error: bad symbolic reference. A signature in KinesisInputDStream.class refers to term InterfaceStability
    in package org.apache.spark.annotation which is not available.
    It may be completely missing from the current classpath, or the version on
    the classpath might be incompatible with the version used when compiling KinesisInputDStream.class.
    
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #81530 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81530/testReport)** for PR 18029 at commit [`cef5cde`](https://github.com/apache/spark/commit/cef5cdece2bd2a7c95e19493c511d602c1b46461).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77700 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77700/testReport)** for PR 18029 at commit [`3cac39b`](https://github.com/apache/spark/commit/3cac39b49b7b56547eee117f53d0182798270111).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @brkyvz Squashed multiple commits into one for better readability. Please have a look when you get time. Thanks.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82624 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82624/testReport)** for PR 18029 at commit [`05cad04`](https://github.com/apache/spark/commit/05cad04d742811689f2f5da7685244bf5c5937a5).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @brkyvz could you please have a look if it looks good. Would be great if you're happy with the changes and we could merge it.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77080/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77136 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77136/testReport)** for PR 18029 at commit [`ff9a586`](https://github.com/apache/spark/commit/ff9a58669853ae0508d3ef599947d15a92e1f712).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82626 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82626/testReport)** for PR 18029 at commit [`483a697`](https://github.com/apache/spark/commit/483a6979d8e06ed99cd1d2deadefdd980bf1ffdf).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77185 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77185/testReport)** for PR 18029 at commit [`734823b`](https://github.com/apache/spark/commit/734823bc4baaf01ff72a493a54344569fc4ae68e).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77726/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77080 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77080/testReport)** for PR 18029 at commit [`9944da8`](https://github.com/apache/spark/commit/9944da82b0b07642f0489c597d9b63176a361f0e).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][DStream] Add changes to use kinesis fetche...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @budde @brkyvz - could I get some love here please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77075 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77075/testReport)** for PR 18029 at commit [`75d8523`](https://github.com/apache/spark/commit/75d852384f12554c3171513f11d31604ff206dac).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137086237
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest: InitialPosition = Latest
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.TRIM_HORIZON.
    +   * @return [[TrimHorizon]]
    +   */
    +  val trimHorizon: InitialPosition = TrimHorizon
    +
    +  /**
    +   * Returns instance of AtTimestamp with InitialPositionInStream.AT_TIMESTAMP.
    +   * @return [[AtTimestamp]]
    +   */
    +  def atTimestamp(timestamp: Date): InitialPosition = AtTimestamp(timestamp)
    +
    +  /**
    +   * Returns instance of [[InitialPosition]] based on the passed [[InitialPositionInStream]].
    +   * This method is used in KinesisUtils for translating the InitialPositionInStream
    +   * to InitialPosition. This function would be removed when we deprecate the KinesisUtils.
    +   *
    +   * @return [[InitialPosition]]
    +   */
    +  def kinesisInitialPositionInStream(
    +    initialPositionInStream: InitialPositionInStream): InitialPosition = {
    +    if (initialPositionInStream == InitialPositionInStream.LATEST) {
    +      latest
    +    } else if (initialPositionInStream == InitialPositionInStream.TRIM_HORIZON) {
    +      trimHorizon
    +    } else {
    +      // InitialPositionInStream.AT_TIMESTAMP is not supported.
    +      // Use InitialPosition.atTimestamp(timestamp) instead.
    +      throw new UnsupportedOperationException(
    +        "Only InitialPositionInStream.LATEST and InitialPositionInStream.TRIM_HORIZON" +
    +          "supported in initialPositionInStream(). Use InitialPosition.atTimestamp(timestamp)" +
    --- End diff --
    
    Rather, `please use the builder API in ... to use AT_TIMESTAMP` 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #79382 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79382/testReport)** for PR 18029 at commit [`a92e0da`](https://github.com/apache/spark/commit/a92e0dac671e4707044f0a9f4512e585518523c9).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r120235619
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -193,6 +197,21 @@ object KinesisInputDStream {
         }
     
         /**
    +     * Sets the Kinesis initial position data to the provided timestamp.
    +     * Sets InitialPositionInStream to [[InitialPositionInStream.AT_TIMESTAMP]]
    +     * and the timestamp to the provided value.
    +     *
    +     * @param timestamp Timestamp to resume the Kinesis stream from a provided
    +     *                  timestamp.
    +     * @return Reference to this [[KinesisInputDStream.Builder]]
    +     */
    +    def withTimestampAtInitialPositionInStream(timestamp: Date) : Builder = {
    --- End diff --
    
    @brkyvz 
    `withInitialPositionAtTimestamp` is an enhancer method for the InitialPositionAtTimestamp. If provided It will set the timestamp value along with the InitialPosition.AT_TIMESTAMP.
    
    Its optional, hence the `initialPositionInStream` can still be used. This will not introduce and incompatibilities in usage. 
    Thoughts ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77701 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77701/testReport)** for PR 18029 at commit [`f1ffcbd`](https://github.com/apache/spark/commit/f1ffcbd897f5a9f6408458c55dd2b7b63ae73b93).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80871/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #83144 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83144/testReport)** for PR 18029 at commit [`2c2a56a`](https://github.com/apache/spark/commit/2c2a56a5c8c7dc4660af77757d26c627ca639e8a).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class KinesisInitialPosition `
      * `sealed trait InitialPosition `
      * `case class AtTimestamp(timestamp: Date) extends InitialPosition `


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85168/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Hi @srowen  we've iterated this patch to bring it in a good state. Need a committer ✔️ before we can go ahead merging it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80913/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77125/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77136/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r120234200
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -100,6 +103,7 @@ object KinesisInputDStream {
         private var endpointUrl: Option[String] = None
         private var regionName: Option[String] = None
         private var initialPositionInStream: Option[InitialPositionInStream] = None
    +    private var initialPositionInStreamTimestamp: Option[Date] = None
    --- End diff --
    
    @brkyvz Where exactly are we planning to add these changes. Are you proposing to change the type of 
    `private var initialPositionInStreamTimestamp: Option[Date] = None`
    
    That would introduce a backward incompatibility on the current builder ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134120475
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest : InitialPosition = Latest
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.TRIM_HORIZON.
    +   * @return [[TrimHorizon]]
    +   */
    +  val trimHorizon : InitialPosition = TrimHorizon
    --- End diff --
    
    *nit* remove space before ```:```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77140 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77140/testReport)** for PR 18029 at commit [`b7f22e4`](https://github.com/apache/spark/commit/b7f22e499a4ece69a5960a14b045ed7874161367).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77125 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77125/testReport)** for PR 18029 at commit [`424550c`](https://github.com/apache/spark/commit/424550c8450937f78ce608ff7b18e46f41478a8a).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77121/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r156265997
  
    --- Diff: external/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisInputDStreamBuilderSuite.scala ---
    @@ -101,12 +102,60 @@ class KinesisInputDStreamBuilderSuite extends TestSuiteBase with BeforeAndAfterE
           .build()
         assert(dstream.endpointUrl == customEndpointUrl)
         assert(dstream.regionName == customRegion)
    -    assert(dstream.initialPositionInStream == customInitialPosition)
    +    assert(dstream.initialPosition == customInitialPosition)
         assert(dstream.checkpointAppName == customAppName)
         assert(dstream.checkpointInterval == customCheckpointInterval)
         assert(dstream._storageLevel == customStorageLevel)
         assert(dstream.kinesisCreds == customKinesisCreds)
         assert(dstream.dynamoDBCreds == Option(customDynamoDBCreds))
         assert(dstream.cloudWatchCreds == Option(customCloudWatchCreds))
    +
    +    // Testing with AtTimestamp
    +    val cal = Calendar.getInstance()
    +    cal.add(Calendar.DATE, -1)
    +    val timestamp = cal.getTime()
    +    val initialPositionAtTimestamp = AtTimestamp(timestamp)
    +
    +    val dstreamAtTimestamp = builder
    +      .endpointUrl(customEndpointUrl)
    +      .regionName(customRegion)
    +      .initialPosition(initialPositionAtTimestamp)
    +      .checkpointAppName(customAppName)
    +      .checkpointInterval(customCheckpointInterval)
    +      .storageLevel(customStorageLevel)
    +      .kinesisCredentials(customKinesisCreds)
    +      .dynamoDBCredentials(customDynamoDBCreds)
    +      .cloudWatchCredentials(customCloudWatchCreds)
    +      .build()
    +    assert(dstreamAtTimestamp.endpointUrl == customEndpointUrl)
    +    assert(dstreamAtTimestamp.regionName == customRegion)
    +    assert(dstreamAtTimestamp.initialPosition.initialPositionInStream
    +      == initialPositionAtTimestamp.initialPositionInStream)
    +    assert(
    +      dstreamAtTimestamp.initialPosition.asInstanceOf[AtTimestamp].timestamp.equals(timestamp))
    +    assert(dstreamAtTimestamp.checkpointAppName == customAppName)
    +    assert(dstreamAtTimestamp.checkpointInterval == customCheckpointInterval)
    +    assert(dstreamAtTimestamp._storageLevel == customStorageLevel)
    +    assert(dstreamAtTimestamp.kinesisCreds == customKinesisCreds)
    +    assert(dstreamAtTimestamp.dynamoDBCreds == Option(customDynamoDBCreds))
    +    assert(dstreamAtTimestamp.cloudWatchCreds == Option(customCloudWatchCreds))
    +
    +    // Testing with AtTimestamp
    +    val initialPositionAtTimestamp2 = AtTimestamp(timestamp)
    --- End diff --
    
    how is the following lines a different test?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #83144 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83144/testReport)** for PR 18029 at commit [`2c2a56a`](https://github.com/apache/spark/commit/2c2a56a5c8c7dc4660af77757d26c627ca639e8a).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82623/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82625 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82625/testReport)** for PR 18029 at commit [`06e3969`](https://github.com/apache/spark/commit/06e3969d0d4e19fb2fb989973a7836ec80f27d82).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137237166
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    --- End diff --
    
    It was required for the Java Api for using `TrimHorizon.instance()`.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137343555
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest: InitialPosition = Latest
    --- End diff --
    
    Good question. They used to be just classes. Since we couldn't have a nice way for using a `case object` in Java, you need to add a $ after the classname, e.g. `TrimHorizon$`, we decided to go the class syntax'y way. In scala, this still allows you to use the class with `KinesisInitialPosition.Latest` for example.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    I really appreciate you taking time to review the changes. I know balancing work and foss contributions is difficult :)
    I have given a clean new stab at the issue with all the feedbacks. Let me know if it looks good.
    GodSpeed @budde.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][DStream] Add changes to use kinesis fetche...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Would love to work on the review feedback here @budde @brkyvz  . Please have a look when you have time.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #85373 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85373/testReport)** for PR 18029 at commit [`bdd080c`](https://github.com/apache/spark/commit/bdd080c2ea7b3c2f215e1ca6a6614ade3f074601).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Thanks @budde for the review . Love the suggestions. 👍 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Could I get some love here from the committers please @brkyvz @HyukjinKwon @srowen . Would love to work on any changes if required.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #79395 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79395/testReport)** for PR 18029 at commit [`b3759dd`](https://github.com/apache/spark/commit/b3759ddce8859df874ae3ae115cdafe40339cbf2).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82635 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82635/testReport)** for PR 18029 at commit [`72703a0`](https://github.com/apache/spark/commit/72703a072e34407b52d555bab98a435414d2ed25).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #79382 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79382/testReport)** for PR 18029 at commit [`a92e0da`](https://github.com/apache/spark/commit/a92e0dac671e4707044f0a9f4512e585518523c9).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `sealed trait InitialPosition `
      * `case class AtTimestamp(timestamp: Date) extends InitialPosition `


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r156266783
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,82 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    --- End diff --
    
    can you wrap all of these in an object?
    ```scala
    sealed trait InitialPosition {
      ...
    }
    
    object internal {
      case object Latest extends InitialPosition {
      }
      ...
      case class AtTimestamp(timestamp: Date) extends InitialPosition {
      }
    }
    ```
    Note how InitialPosition is outside, and `internal` is lowercase.
    
    so that people go only through the Java Interface (`org.apache.spark.streaming.kinesis.Latest()`) etc
    
    Your documentation and test cases go through the Scala interface which makes it super weird to have 2 things corresponding to the same thing.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Hi, @budde , @brkyvz would love to hear your thoughts on the new patch


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #85373 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85373/testReport)** for PR 18029 at commit [`bdd080c`](https://github.com/apache/spark/commit/bdd080c2ea7b3c2f215e1ca6a6614ade3f074601).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class KinesisInitialPositions `
      * `    public static class Latest implements KinesisInitialPosition, Serializable `
      * `    public static class TrimHorizon implements KinesisInitialPosition, Serializable `
      * `    public static class AtTimestamp implements KinesisInitialPosition, Serializable `


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r143849691
  
    --- Diff: external/kinesis-asl/src/main/java/org/apache/spark/streaming/kinesis/KinesisInitialPosition.java ---
    @@ -0,0 +1,53 @@
    +/*
    + * 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.spark.streaming.kinesis;
    +
    +import java.util.Date;
    +
    +/**
    + * A java wrapper for org.apache.spark.streaming.kinesis.InitialPosition
    + * to expose the corresponding scala objects for InitialPositionInStream.
    + * The functions are intentionally Upper cased to appear like classes for
    + * usage in Java classes.
    + */
    +public class KinesisInitialPosition {
    +
    +    /**
    +     * Returns instance of AtTimestamp with InitialPositionInStream.LATEST.
    +     * @return
    +     */
    +    public static InitialPosition Latest() {
    +        return InitialPosition$.MODULE$.latest();
    +    }
    +
    +    /**
    +     * Returns instance of AtTimestamp with InitialPositionInStream.TRIM_HORIZON.
    +     * @return
    +     */
    +    public static InitialPosition TrimHorizon() {
    +        return InitialPosition$.MODULE$.trimHorizon();
    +    }
    +
    +    /**
    +     * Returns instance of AtTimestamp with InitialPositionInStream.AT_TIMESTAMP.
    +     * @param timestamp
    +     * @return
    +     */
    +    public static InitialPosition AtTimestamp(Date timestamp) {
    +        return InitialPosition$.MODULE$.atTimestamp(timestamp);
    --- End diff --
    
    `AtTimestamp.apply(timestamp)`


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77043/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77726 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77726/testReport)** for PR 18029 at commit [`5d9bec3`](https://github.com/apache/spark/commit/5d9bec38e3f4fa1d1c43a30b3d4c54341c49ec17).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @budde @brkyvz - Would appreciate if you could share some tips by which I could include these external kinesis modules on spark-shell to test this directly from shell (both for scala and python) ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #79395 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79395/testReport)** for PR 18029 at commit [`b3759dd`](https://github.com/apache/spark/commit/b3759ddce8859df874ae3ae115cdafe40339cbf2).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `sealed trait InitialPosition `
      * `case class AtTimestamp(timestamp: Date) extends InitialPosition `


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @budde  What would that mean for existing applications that are sending an `InitialPositionInStream.SOMETHING` to the the builder. We would need to provide multiple overloaded versions of the `builder.initialPositionInStream()` so that we don't mess them up, and the appropriate method populates the new `InitialPosition ` argument and pass it down to `KinesisReceiver`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77075 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77075/testReport)** for PR 18029 at commit [`75d8523`](https://github.com/apache/spark/commit/75d852384f12554c3171513f11d31604ff206dac).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134067523
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,107 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  var initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * Returns instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  def latest() : InitialPosition = {
    --- End diff --
    
    I'd just make this a ```val``` or at least remove the parens as this method has no side-effects


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82623 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82623/testReport)** for PR 18029 at commit [`107c1ab`](https://github.com/apache/spark/commit/107c1aba8b029cac933ab726e45e9f13fabcfccb).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #80871 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80871/testReport)** for PR 18029 at commit [`c3622b9`](https://github.com/apache/spark/commit/c3622b91332775a363c68d9571f4f5dbc3af84fc).
     * This patch **fails to build**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yashs360 <gi...@git.apache.org>.
Github user yashs360 commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Hi @brkyvz Thinking on these lines, Adding them as Java objects adds more complexity to our design. We again have to think about making the objects singleton and thread safe. The Scala case class were very simple and minimal.
    
    This is how we would have to implement the java classes for the Initial positions. It looks a bit unclean to me. Thoughts ?
    
    ```
    
    abstract class InitialPosition {
        public static final InitialPositionInStream initialPositionInStream = InitialPositionInStream.LATEST;
    }
    
    class Latest extends InitialPosition {
        private static final Latest instance = new Latest();
        static final InitialPositionInStream initialPositionInStream = InitialPositionInStream.LATEST;
    
        private Latest(){}
    
        public static InitialPosition getInstance() {
            return instance;
        }
    }
    
    class TrimHorizon extends InitialPosition {
        private static final TrimHorizon instance = new TrimHorizon();
        static final InitialPositionInStream initialPositionInStream = InitialPositionInStream.TRIM_HORIZON;
    
        private TrimHorizon(){}
    
        public static InitialPosition getInstance() {
            return instance;
        }
    }
    
    class AtTimestamp extends InitialPosition {
        static final InitialPositionInStream initialPositionInStream = InitialPositionInStream.AT_TIMESTAMP;
        Date timestamp;
    
        private AtTimestamp(Date timestamp){
            this.timestamp = timestamp;
        }
    
        public static InitialPosition getInstance(Date timestamp) {
            return new AtTimestamp(timestamp);
        }
    }
    
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #85181 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85181/testReport)** for PR 18029 at commit [`3c16c47`](https://github.com/apache/spark/commit/3c16c478257c8aed61b1cef4d75360b8bb8b166d).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class KinesisInitialPositions `
      * `    public static class Latest implements KinesisInitialPosition, Serializable `
      * `    public static class TrimHorizon implements KinesisInitialPosition, Serializable `
      * `    public static class AtTimestamp implements KinesisInitialPosition, Serializable `


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82708/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @brkyvz Could you please check this for the last suggestions ?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged to master. Thanks!


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137235013
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest: InitialPosition = Latest
    --- End diff --
    
    Interesting. Could you please explain why have we done this capitalization. Once() and ProcessingTime() are methods and shouldn't they be camel cased ?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @brkyvz Please have a look once you have time. Thanks.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82708 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82708/testReport)** for PR 18029 at commit [`ac0ae53`](https://github.com/apache/spark/commit/ac0ae53935510221ebb6a44f6ec30b68fca34343).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Thanks @brkyvz . Appreciate the review comments.
    I am facing some issues with the scala packaging. I will keep working on it and update the patch soon.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Thanks for the review @brkyvz .
    Please have a look at the new PR. I have implemented the review comments.
    For the API hygiene, I have added both the new and old API, and have marked the old API as deprecated. The rational behind keeping the new API is to support any future changes to the Kinesis consumer lib, and we could just add new case classes in one place to handle new params. Otherwise we would have to keep adding new arguments to the kinesis receiver function. The New api also makes it clear to understand what exactly is the purpose of the arguments.
    I have also added new test cases to test the old api with both +ve and -ve scenarios.
    
    Please have a look and share your thoughts.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77775/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Thoughts - @budde @brkyvz ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r119723361
  
    --- Diff: external/kinesis-asl/src/test/java/org/apache/spark/streaming/kinesis/JavaKinesisInputDStreamBuilderSuite.java ---
    @@ -57,6 +58,26 @@ public void testJavaKinesisDStreamBuilder() {
         assert(kinesisDStream.checkpointAppName() == appName);
         assert(kinesisDStream.checkpointInterval() == checkpointInterval);
         assert(kinesisDStream._storageLevel() == storageLevel);
    +
    +    Date yesterday = DateUtils.addDays(new Date(), -1);
    +    KinesisInputDStream<byte[]> kinesisDStreamFromTimestamp = KinesisInputDStream.builder()
    +            .streamingContext(ssc)
    +            .streamName(streamName)
    +            .endpointUrl(endpointUrl)
    +            .regionName(region)
    +            .initialPositionInStream(yesterday)
    +            .checkpointAppName(appName)
    +            .checkpointInterval(checkpointInterval)
    +            .storageLevel(storageLevel)
    +            .build();
    --- End diff --
    
    Indentation is inconsistent with lines 45-53


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #80913 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80913/testReport)** for PR 18029 at commit [`eb7ad56`](https://github.com/apache/spark/commit/eb7ad56b598af5e537e5fa1808dc93b692a14f6f).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    For review @budde @brkyvz ^ this looks in a good state.
    @brkyvz - I still didn't quite get the code bit you shared, would love to make changes if you could explain that piece of your mind please.
    
    Where exactly would this part fit-
    ```
    trait InitialPosition {
      def setInitialPosition(clientConf: KinesisClientLibConfiguration): KinesisClientLibConfiguration
    }
    
    case object Latest {
      override def setInitialPosition(clientConf: KinesisClientLibConfiguration): KinesisClientLibConfiguration = {
        clientLibConf.withInitialPositionInStream(InitialPositionInStream.LATEST)
      }
    }
    
    case object TrimHorizon {
      override def setInitialPosition(clientConf: KinesisClientLibConfiguration): KinesisClientLibConfiguration = {
        clientLibConf.withInitialPositionInStream(InitialPositionInStream.TRIM_HORIZON)
      }
    }
    
    case class AtTimestamp(timestamp: Date) {
      override def setInitialPosition(clientConf: KinesisClientLibConfiguration): KinesisClientLibConfiguration = {
        clientLibConf. withTimestampAtInitialPositionInStream(timestamp)
      }
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77125 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77125/testReport)** for PR 18029 at commit [`424550c`](https://github.com/apache/spark/commit/424550c8450937f78ce608ff7b18e46f41478a8a).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134067791
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -308,7 +308,6 @@ object KinesisInputDStream {
       private[kinesis] val DEFAULT_KINESIS_ENDPOINT_URL: String =
         "https://kinesis.us-east-1.amazonaws.com"
       private[kinesis] val DEFAULT_KINESIS_REGION_NAME: String = "us-east-1"
    -  private[kinesis] val DEFAULT_INITIAL_POSITION_IN_STREAM: InitialPositionInStream =
    -    InitialPositionInStream.LATEST
    +  private[kinesis] val DEFAULT_INITIAL_POSITION_IN_STREAM: InitialPosition = InitialPosition.latest
    --- End diff --
    
    *nit* Rename this to ```DEFAULT_INITIAL_POSITION``` to reflect the new class name


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77775 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77775/testReport)** for PR 18029 at commit [`c4ed442`](https://github.com/apache/spark/commit/c4ed442eb20f56cb346cfb42234f2652ab1883e3).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r156266325
  
    --- Diff: external/kinesis-asl/src/main/java/org/apache/spark/streaming/kinesis/KinesisInitialPosition.java ---
    @@ -0,0 +1,53 @@
    +/*
    + * 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.spark.streaming.kinesis;
    +
    +import java.util.Date;
    +
    +/**
    + * A java wrapper for org.apache.spark.streaming.kinesis.InitialPosition
    + * to expose the corresponding scala objects for InitialPositionInStream.
    + * The functions are intentionally Upper cased to appear like classes for
    + * usage in Java classes.
    + */
    +public class KinesisInitialPosition {
    --- End diff --
    
    please add `@InterfaceStability.Evolving` above


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134120535
  
    --- Diff: external/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisInputDStreamBuilderSuite.scala ---
    @@ -72,7 +70,8 @@ class KinesisInputDStreamBuilderSuite extends TestSuiteBase with BeforeAndAfterE
         val dstream = builder.build()
         assert(dstream.endpointUrl == DEFAULT_KINESIS_ENDPOINT_URL)
         assert(dstream.regionName == DEFAULT_KINESIS_REGION_NAME)
    -    assert(dstream.initialPositionInStream == DEFAULT_INITIAL_POSITION_IN_STREAM)
    +    assert(dstream.initialPosition.initialPositionInStream
    --- End diff --
    
    *nit* Think this would be sufficient:
    
    ```scala
    assert(dstream.initialPosition == DEFAULT_INITIAL_POSITION)
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77796 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77796/testReport)** for PR 18029 at commit [`8c380d8`](https://github.com/apache/spark/commit/8c380d8b002cbb8ec9223bc51571f6b87d938dba).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77739/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77140/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r158627994
  
    --- Diff: external/kinesis-asl/src/test/java/org/apache/spark/streaming/kinesis/JavaKinesisInputDStreamBuilderSuite.java ---
    @@ -45,18 +44,90 @@ public void testJavaKinesisDStreamBuilder() {
           .streamName(streamName)
           .endpointUrl(endpointUrl)
           .regionName(region)
    -      .initialPositionInStream(initialPosition)
    +      .initialPosition(initialPosition)
           .checkpointAppName(appName)
           .checkpointInterval(checkpointInterval)
           .storageLevel(storageLevel)
           .build();
         assert(kinesisDStream.streamName() == streamName);
         assert(kinesisDStream.endpointUrl() == endpointUrl);
         assert(kinesisDStream.regionName() == region);
    -    assert(kinesisDStream.initialPositionInStream() == initialPosition);
    +    assert(kinesisDStream.initialPosition().getPosition() == initialPosition.getPosition());
    +    assert(kinesisDStream.checkpointAppName() == appName);
    +    assert(kinesisDStream.checkpointInterval() == checkpointInterval);
    +    assert(kinesisDStream._storageLevel() == storageLevel);
    +    ssc.stop();
    +  }
    +
    +  /**
    +   * Test to ensure that the old API for InitialPositionInStream
    +   * is supported in KinesisDStream.Builder.
    +   * This test would be removed when we deprecate the KinesisUtils.
    +   */
    +  @Test
    +  public void testJavaKinesisDStreamBuilderOldApi() {
    +    String streamName = "a-very-nice-stream-name";
    +    String endpointUrl = "https://kinesis.us-west-2.amazonaws.com";
    +    String region = "us-west-2";
    +    String appName = "a-very-nice-kinesis-app";
    +    Duration checkpointInterval = Seconds.apply(30);
    +    StorageLevel storageLevel = StorageLevel.MEMORY_ONLY();
    +
    +    KinesisInputDStream<byte[]> kinesisDStream = KinesisInputDStream.builder()
    +            .streamingContext(ssc)
    +            .streamName(streamName)
    +            .endpointUrl(endpointUrl)
    +            .regionName(region)
    +            .initialPositionInStream(InitialPositionInStream.LATEST)
    +            .checkpointAppName(appName)
    +            .checkpointInterval(checkpointInterval)
    +            .storageLevel(storageLevel)
    +            .build();
    +    assert(kinesisDStream.streamName() == streamName);
    +    assert(kinesisDStream.endpointUrl() == endpointUrl);
    +    assert(kinesisDStream.regionName() == region);
    +    assert(kinesisDStream.initialPosition().getPosition() == InitialPositionInStream.LATEST);
         assert(kinesisDStream.checkpointAppName() == appName);
         assert(kinesisDStream.checkpointInterval() == checkpointInterval);
         assert(kinesisDStream._storageLevel() == storageLevel);
         ssc.stop();
       }
    +
    +  /**
    +   * Test to ensure that the old API for InitialPositionInStream
    +   * is supported in KinesisDStream.Builder.
    +   * Test old API doesn't support the InitialPositionInStream.AT_TIMESTAMP.
    +   * This test would be removed when we deprecate the KinesisUtils.
    +   */
    +  @Test
    +  public void testJavaKinesisDStreamBuilderOldApiAtTimestamp() {
    --- End diff --
    
    This test could be moved to become a Scala test instead, using 
    ```scala
    intercept[UnsupportedOperationException] {
      ...
    }
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77727/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79383/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r120180756
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -100,6 +103,7 @@ object KinesisInputDStream {
         private var endpointUrl: Option[String] = None
         private var regionName: Option[String] = None
         private var initialPositionInStream: Option[InitialPositionInStream] = None
    +    private var initialPositionInStreamTimestamp: Option[Date] = None
    --- End diff --
    
    umm, I feel a better way would be like:
    ```scala
    trait InitialPosition {
      def setInitialPosition(clientConf: KinesisClientLibConfiguration): KinesisClientLibConfiguration
    }
    
    case object Latest {
      override def setInitialPosition(clientConf: KinesisClientLibConfiguration): KinesisClientLibConfiguration = {
        clientLibConf.withInitialPositionInStream(InitialPositionInStream.LATEST)
      }
    }
    
    case object TrimHorizon {
      override def setInitialPosition(clientConf: KinesisClientLibConfiguration): KinesisClientLibConfiguration = {
        clientLibConf.withInitialPositionInStream(InitialPositionInStream.TRIM_HORIZON)
      }
    }
    
    case class AtTimestamp(timestamp: Date) {
      override def setInitialPosition(clientConf: KinesisClientLibConfiguration): KinesisClientLibConfiguration = {
        clientLibConf. withTimestampAtInitialPositionInStream(timestamp)
      }
    }
    ```
    
    what do you think?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #80876 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80876/testReport)** for PR 18029 at commit [`7d9a08a`](https://github.com/apache/spark/commit/7d9a08aa4471e2b76bb15e72325ef717ab469ee2).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `sealed trait InitialPosition `
      * `case class AtTimestamp(timestamp: Date) extends InitialPosition `


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r143849634
  
    --- Diff: external/kinesis-asl/src/main/java/org/apache/spark/streaming/kinesis/KinesisInitialPosition.java ---
    @@ -0,0 +1,53 @@
    +/*
    + * 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.spark.streaming.kinesis;
    +
    +import java.util.Date;
    +
    +/**
    + * A java wrapper for org.apache.spark.streaming.kinesis.InitialPosition
    + * to expose the corresponding scala objects for InitialPositionInStream.
    + * The functions are intentionally Upper cased to appear like classes for
    + * usage in Java classes.
    + */
    +public class KinesisInitialPosition {
    +
    +    /**
    +     * Returns instance of AtTimestamp with InitialPositionInStream.LATEST.
    +     * @return
    +     */
    +    public static InitialPosition Latest() {
    +        return InitialPosition$.MODULE$.latest();
    +    }
    +
    +    /**
    +     * Returns instance of AtTimestamp with InitialPositionInStream.TRIM_HORIZON.
    +     * @return
    +     */
    +    public static InitialPosition TrimHorizon() {
    +        return InitialPosition$.MODULE$.trimHorizon();
    --- End diff --
    
    Can you instead return `TrimHorizon$.MODULE$`


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Will update and post another request seen. Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77700/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79395/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77140 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77140/testReport)** for PR 18029 at commit [`b7f22e4`](https://github.com/apache/spark/commit/b7f22e499a4ece69a5960a14b045ed7874161367).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Actually yeah, I like your way.
    
    On Dec 14, 2017 3:08 PM, "yashs360" <no...@github.com> wrote:
    
    > *@yashs360* commented on this pull request.
    > ------------------------------
    >
    > In external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/
    > InitialPosition.scala
    > <https://github.com/apache/spark/pull/18029#discussion_r157086878>:
    >
    > > +import java.util.Date
    > +
    > +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    > +
    > +/**
    > + * Trait for Kinesis's InitialPositionInStream.
    > + * This will be overridden by more specific types.
    > + */
    > +sealed trait InitialPosition {
    > +  val initialPositionInStream: InitialPositionInStream
    > +}
    > +
    > +/**
    > + * Case object for Kinesis's InitialPositionInStream.LATEST.
    > + */
    > +case object Latest extends InitialPosition {
    >
    > Hi @brkyvz <https://github.com/brkyvz> , Thanks for the review.
    > Are you suggesting to put everything into a new object. And refer the case
    > objects from the java class methods?
    > In that case is it better to create the objects in Java and expose them
    > directly, since we will have cases where we will need direct access to the
    > case objects/classes (instead of the java methods) like one of the test
    > cases:
    > initialPosition.asInstanceOf[AtTimestamp].timestamp
    >
    > I would create a new branch with the changes and share with you if its
    > fine ?
    >
    > —
    > You are receiving this because you were mentioned.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/spark/pull/18029#discussion_r157086878>, or mute
    > the thread
    > <https://github.com/notifications/unsubscribe-auth/AFACewoTV1GYt4dpddBP_Jsx7cF6AUVjks5tAaprgaJpZM4NfLn->
    > .
    >



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    My understanding of @brkyvz's suggestion is that you remove the two ```initialPositionInStream``` and ```initialPositionInStreamTimestamp``` arguments in favor of the single ```InitialPosition``` interface that can represent all possible combinations of Kinesis initial positions. The builder could construct the appropriate ```InitialPosition``` argument to pass to the stream depending on the parameters provided to the builder.
    
    I don't have strong opinions on where the interface and case classes/objects should be defined but I think creating a new source file à la SparkAWSCredentials.scala would be fine. I think making ```InitialPosition``` a sealed trait would also make sense logically but that probably isn't super important.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r158627941
  
    --- Diff: external/kinesis-asl/src/test/java/org/apache/spark/streaming/kinesis/JavaKinesisInputDStreamBuilderSuite.java ---
    @@ -45,18 +44,90 @@ public void testJavaKinesisDStreamBuilder() {
           .streamName(streamName)
           .endpointUrl(endpointUrl)
           .regionName(region)
    -      .initialPositionInStream(initialPosition)
    +      .initialPosition(initialPosition)
           .checkpointAppName(appName)
           .checkpointInterval(checkpointInterval)
           .storageLevel(storageLevel)
           .build();
         assert(kinesisDStream.streamName() == streamName);
         assert(kinesisDStream.endpointUrl() == endpointUrl);
         assert(kinesisDStream.regionName() == region);
    -    assert(kinesisDStream.initialPositionInStream() == initialPosition);
    +    assert(kinesisDStream.initialPosition().getPosition() == initialPosition.getPosition());
    +    assert(kinesisDStream.checkpointAppName() == appName);
    +    assert(kinesisDStream.checkpointInterval() == checkpointInterval);
    +    assert(kinesisDStream._storageLevel() == storageLevel);
    +    ssc.stop();
    +  }
    +
    +  /**
    +   * Test to ensure that the old API for InitialPositionInStream
    +   * is supported in KinesisDStream.Builder.
    +   * This test would be removed when we deprecate the KinesisUtils.
    +   */
    +  @Test
    +  public void testJavaKinesisDStreamBuilderOldApi() {
    +    String streamName = "a-very-nice-stream-name";
    +    String endpointUrl = "https://kinesis.us-west-2.amazonaws.com";
    +    String region = "us-west-2";
    +    String appName = "a-very-nice-kinesis-app";
    +    Duration checkpointInterval = Seconds.apply(30);
    +    StorageLevel storageLevel = StorageLevel.MEMORY_ONLY();
    +
    +    KinesisInputDStream<byte[]> kinesisDStream = KinesisInputDStream.builder()
    +            .streamingContext(ssc)
    --- End diff --
    
    nit: indentation should be 2 spaces.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #80913 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80913/testReport)** for PR 18029 at commit [`eb7ad56`](https://github.com/apache/spark/commit/eb7ad56b598af5e537e5fa1808dc93b692a14f6f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `sealed trait InitialPosition `
      * `case class AtTimestamp(timestamp: Date) extends InitialPosition `


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r120234538
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -100,6 +103,7 @@ object KinesisInputDStream {
         private var endpointUrl: Option[String] = None
         private var regionName: Option[String] = None
         private var initialPositionInStream: Option[InitialPositionInStream] = None
    +    private var initialPositionInStreamTimestamp: Option[Date] = None
    --- End diff --
    
    I'm hoping we won't have to take both `initialPositionInStream` and `initialPositionInStreamTimestamp`. The builder is internal APIs, therefore we can definitely change it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @yssharma Let me know what you think of my review suggestions. I should be able to review any updates from here on in a timely manner but you will still need @brkyvz or another Spark committer to do the final review and approve the PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #81358 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81358/testReport)** for PR 18029 at commit [`0c46008`](https://github.com/apache/spark/commit/0c4600818b6e248741621d23a748c37c7b7e8c68).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `sealed trait InitialPosition `
      * `case class AtTimestamp(timestamp: Date) extends InitialPosition `


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r119723184
  
    --- Diff: external/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisInputDStreamBuilderSuite.scala ---
    @@ -112,4 +112,38 @@ class KinesisInputDStreamBuilderSuite extends TestSuiteBase with BeforeAndAfterE
         assert(dstream.dynamoDBCreds == Option(customDynamoDBCreds))
         assert(dstream.cloudWatchCreds == Option(customCloudWatchCreds))
       }
    +
    +  test("should propagate kinesis fetch timestamp values to KinesisInputDStream") {
    --- End diff --
    
    Why not just roll this into the previous check? There's a lot of code duplication going on for no other reason than to check that another config value is passed properly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134063380
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,107 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  var initialPositionInStream: InitialPositionInStream
    --- End diff --
    
    This should be a ```val``` or, better yet, a ```def``` (```def``` can be overridden with ```val``` in child classes)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81358/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77796 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77796/testReport)** for PR 18029 at commit [`8c380d8`](https://github.com/apache/spark/commit/8c380d8b002cbb8ec9223bc51571f6b87d938dba).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77185 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77185/testReport)** for PR 18029 at commit [`734823b`](https://github.com/apache/spark/commit/734823bc4baaf01ff72a493a54344569fc4ae68e).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134120473
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest : InitialPosition = Latest
    --- End diff --
    
    *nit* remove space before ```:```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134067537
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,107 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  var initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  def instance: InitialPosition = this
    +  override var initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * Returns instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  def latest() : InitialPosition = {
    +    Latest
    +  }
    +
    +  /**
    +   * Returns instance of Latest with InitialPositionInStream.TRIM_HORIZON.
    +   * @return [[TrimHorizon]]
    +   */
    +  def trimHorizon() : InitialPosition = {
    --- End diff --
    
    Change to ```val```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @brkyvz I have made the suggested modifications to the code. Please have a look when you get time. Thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77043 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77043/testReport)** for PR 18029 at commit [`7cf935b`](https://github.com/apache/spark/commit/7cf935bfd1d0c12f4a9c9477f2af1e68457559e9).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #81530 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81530/testReport)** for PR 18029 at commit [`cef5cde`](https://github.com/apache/spark/commit/cef5cdece2bd2a7c95e19493c511d602c1b46461).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class KinesisInitialPosition `
      * `sealed trait InitialPosition `
      * `case class AtTimestamp(timestamp: Date) extends InitialPosition `


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Could I get some love here please @budde @brkyvz 
    This change is pretty useful for us and I would like to bake this patch well so others can take advantage of this feature.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Resolved conflict introduced by other code commits.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77727 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77727/testReport)** for PR 18029 at commit [`4ba5424`](https://github.com/apache/spark/commit/4ba5424184fd4cf4c6e2aed58ae5352806f549fd).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r119723865
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisReceiver.scala ---
    @@ -148,17 +149,31 @@ private[kinesis] class KinesisReceiver[T](
     
         kinesisCheckpointer = new KinesisCheckpointer(receiver, checkpointInterval, workerId)
         val kinesisProvider = kinesisCreds.provider
    -    val kinesisClientLibConfiguration = new KinesisClientLibConfiguration(
    -          checkpointAppName,
    -          streamName,
    -          kinesisProvider,
    -          dynamoDBCreds.map(_.provider).getOrElse(kinesisProvider),
    -          cloudWatchCreds.map(_.provider).getOrElse(kinesisProvider),
    -          workerId)
    -        .withKinesisEndpoint(endpointUrl)
    -        .withInitialPositionInStream(initialPositionInStream)
    -        .withTaskBackoffTimeMillis(500)
    -        .withRegionName(regionName)
    +
    +    val kinesisClientLibConfiguration = {
    +      var clientLibConf = new KinesisClientLibConfiguration(
    +        checkpointAppName,
    +        streamName,
    +        kinesisProvider,
    +        dynamoDBCreds.map(_.provider).getOrElse(kinesisProvider),
    +        cloudWatchCreds.map(_.provider).getOrElse(kinesisProvider),
    +        workerId)
    +      .withKinesisEndpoint(endpointUrl)
    +      .withTaskBackoffTimeMillis(500)
    +      .withRegionName(regionName)
    +
    +      /** Enhance the Kinesis receiver based on InitialPositionInStream  */
    --- End diff --
    
    I'd use the standard ```// comment``` syntax here instead of JavaDoc as you aren't documenting a class or method.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134120566
  
    --- Diff: external/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisInputDStreamBuilderSuite.scala ---
    @@ -104,12 +103,61 @@ class KinesisInputDStreamBuilderSuite extends TestSuiteBase with BeforeAndAfterE
           .build()
         assert(dstream.endpointUrl == customEndpointUrl)
         assert(dstream.regionName == customRegion)
    -    assert(dstream.initialPositionInStream == customInitialPosition)
    +    assert(dstream.initialPosition.initialPositionInStream
    +      == customInitialPosition.initialPositionInStream)
         assert(dstream.checkpointAppName == customAppName)
         assert(dstream.checkpointInterval == customCheckpointInterval)
         assert(dstream._storageLevel == customStorageLevel)
         assert(dstream.kinesisCreds == customKinesisCreds)
         assert(dstream.dynamoDBCreds == Option(customDynamoDBCreds))
         assert(dstream.cloudWatchCreds == Option(customCloudWatchCreds))
    +
    +    // Testing with InitialPosition.atTimestamp
    +    val cal = Calendar.getInstance()
    +    cal.add(Calendar.DATE, -1)
    +    val timestamp = cal.getTime()
    +    val initialPositionAtTimestamp = InitialPosition.atTimestamp(timestamp)
    +
    +    val dstreamAtTimestamp = builder
    +      .endpointUrl(customEndpointUrl)
    +      .regionName(customRegion)
    +      .initialPosition(initialPositionAtTimestamp)
    +      .checkpointAppName(customAppName)
    +      .checkpointInterval(customCheckpointInterval)
    +      .storageLevel(customStorageLevel)
    +      .kinesisCredentials(customKinesisCreds)
    +      .dynamoDBCredentials(customDynamoDBCreds)
    +      .cloudWatchCredentials(customCloudWatchCreds)
    +      .build()
    +    assert(dstreamAtTimestamp.endpointUrl == customEndpointUrl)
    +    assert(dstreamAtTimestamp.regionName == customRegion)
    +    assert(dstreamAtTimestamp.initialPosition.initialPositionInStream
    --- End diff --
    
    *nit* Again, I think this could be simplified to:
    
    ```scala
    assert(dstream.initialPosition == initialPositionAtTimestamp)
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77726 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77726/testReport)** for PR 18029 at commit [`5d9bec3`](https://github.com/apache/spark/commit/5d9bec38e3f4fa1d1c43a30b3d4c54341c49ec17).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by yashs360 <gi...@git.apache.org>.
Github user yashs360 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r157086878
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,82 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    --- End diff --
    
    Hi @brkyvz , Thanks for the review. 
    Are you suggesting to put everything into a new object. And refer the case objects from the java class methods?
    In that case is it better to create the objects in Java and expose them directly, since we will have cases where we will need direct access to the case objects/classes (instead of the java methods) like one of the test cases:
    `initialPosition.asInstanceOf[AtTimestamp].timestamp`
    
    I would create a new branch with the changes and share with you if its fine ?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/18029


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Commit https://github.com/apache/spark/commit/424550c8450937f78ce608ff7b18e46f41478a8a should fix the timeouts mentioned in the https://github.com/apache/spark/commit/b71a8d621ff048958dd5f10ef16cf5989026ed5f commit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yashs360 <gi...@git.apache.org>.
Github user yashs360 commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Thanks @brkyvz for all your help on this one. I have pushed the changes and waiting for the test build.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r120180952
  
    --- Diff: external/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisInputDStreamBuilderSuite.scala ---
    @@ -111,5 +111,29 @@ class KinesisInputDStreamBuilderSuite extends TestSuiteBase with BeforeAndAfterE
         assert(dstream.kinesisCreds == customKinesisCreds)
         assert(dstream.dynamoDBCreds == Option(customDynamoDBCreds))
         assert(dstream.cloudWatchCreds == Option(customCloudWatchCreds))
    +
    +    val yesterday = DateUtils.addDays(new Date, -1)
    +    val dStreamFromTimestamp = builder
    +    .endpointUrl(customEndpointUrl)
    --- End diff --
    
    please indent all these lines


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #82626 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82626/testReport)** for PR 18029 at commit [`483a697`](https://github.com/apache/spark/commit/483a6979d8e06ed99cd1d2deadefdd980bf1ffdf).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137086738
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    --- End diff --
    
    For these to be Java friendly, we may need to put them in a java file, similar to https://github.com/apache/spark/blob/master/sql/core/src/main/java/org/apache/spark/sql/streaming/Trigger.java


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r143850553
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -182,14 +181,14 @@ object KinesisInputDStream {
     
         /**
          * Sets the initial position data is read from in the Kinesis stream. Defaults to
    -     * [[InitialPositionInStream.LATEST]] if no custom value is specified.
    +     * [[InitialPosition.latest]] if no custom value is specified.
          *
    -     * @param initialPosition InitialPositionInStream value specifying where Spark Streaming
    +     * @param initialPosition [[InitialPosition]] value specifying where Spark Streaming
          *                        will start reading records in the Kinesis stream from
          * @return Reference to this [[KinesisInputDStream.Builder]]
          */
    -    def initialPositionInStream(initialPosition: InitialPositionInStream): Builder = {
    --- End diff --
    
    don't remove this API, since it will break compatibility. Instead add an API to take the `withTimestamp`. In the end if we see that `withTimestamp` has been set, but initial position isn't `AtTimestamp`, then we throw an error. Likewise if `AtTimestamp` is set, but no timestamp has been provided, also throw an error.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @budde @brkyvz would love to hear your thoughts if this is the best way to add this functionality 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r120235938
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -193,6 +197,21 @@ object KinesisInputDStream {
         }
     
         /**
    +     * Sets the Kinesis initial position data to the provided timestamp.
    +     * Sets InitialPositionInStream to [[InitialPositionInStream.AT_TIMESTAMP]]
    +     * and the timestamp to the provided value.
    +     *
    +     * @param timestamp Timestamp to resume the Kinesis stream from a provided
    +     *                  timestamp.
    +     * @return Reference to this [[KinesisInputDStream.Builder]]
    +     */
    +    def withTimestampAtInitialPositionInStream(timestamp: Date) : Builder = {
    --- End diff --
    
    I just suggested renaming it. Sorry for the confusion


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85171/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #81358 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81358/testReport)** for PR 18029 at commit [`0c46008`](https://github.com/apache/spark/commit/0c4600818b6e248741621d23a748c37c7b7e8c68).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r143849596
  
    --- Diff: external/kinesis-asl/src/main/java/org/apache/spark/streaming/kinesis/KinesisInitialPosition.java ---
    @@ -0,0 +1,53 @@
    +/*
    + * 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.spark.streaming.kinesis;
    +
    +import java.util.Date;
    +
    +/**
    + * A java wrapper for org.apache.spark.streaming.kinesis.InitialPosition
    + * to expose the corresponding scala objects for InitialPositionInStream.
    + * The functions are intentionally Upper cased to appear like classes for
    + * usage in Java classes.
    + */
    +public class KinesisInitialPosition {
    +
    +    /**
    +     * Returns instance of AtTimestamp with InitialPositionInStream.LATEST.
    +     * @return
    +     */
    +    public static InitialPosition Latest() {
    +        return InitialPosition$.MODULE$.latest();
    --- End diff --
    
    Can you instead return `Latest$.MODULE$`


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77075/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r134138994
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.LATEST.
    +   * @return [[Latest]]
    +   */
    +  val latest : InitialPosition = Latest
    +
    +  /**
    +   * An instance of Latest with InitialPositionInStream.TRIM_HORIZON.
    +   * @return [[TrimHorizon]]
    +   */
    +  val trimHorizon : InitialPosition = TrimHorizon
    +
    +  /**
    +   * Returns instance of AtTimestamp with InitialPositionInStream.AT_TIMESTAMP.
    +   * @return [[AtTimestamp]]
    +   */
    +  def atTimestamp(timestamp: Date) : InitialPosition = AtTimestamp(timestamp)
    +
    +  /**
    +   * Returns instance of [[InitialPosition]] based on the passed [[InitialPositionInStream]].
    +   * This method is used in KinesisUtils for translating the InitialPositionInStream
    +   * to InitialPosition. This function would be removed when we deprecate the KinesisUtils.
    +   *
    +   * @return [[InitialPosition]]
    +   */
    +  def kinesisInitialPositionInStream(
    +    initialPositionInStream: InitialPositionInStream) : InitialPosition = {
    --- End diff --
    
    Added all other review comments. The indentation was making it look weird, so skipped the indentation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r119984045
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -38,6 +40,7 @@ private[kinesis] class KinesisInputDStream[T: ClassTag](
         val endpointUrl: String,
         val regionName: String,
         val initialPositionInStream: InitialPositionInStream,
    +    val initialPositionInStreamTimestamp: Date,
    --- End diff --
    
    @budde - I had two approaches in mind while adding this functionality-
    1. Additional parameter which can be set by an overloaded method in Builder.
    2. Creating a new case class for wrapping initial position with an optional timestamp.
    
    I went ahead with implementing the first one for backward compatibility such that users can use their same builders.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Simplified how timestamp is passed to Kinesis for InitialPositionInStream.AT_TIMESTAMP.
    cc: @budde , @brkyvz 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168] [DStream] Add changes to use kinesi...

Posted by brkyvz <gi...@git.apache.org>.
Github user brkyvz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r137342850
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/InitialPosition.scala ---
    @@ -0,0 +1,104 @@
    +/*
    + * 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.spark.streaming.kinesis
    +
    +import java.util.Date
    +
    +import com.amazonaws.services.kinesis.clientlibrary.lib.worker.InitialPositionInStream
    +
    +/**
    + * Trait for Kinesis's InitialPositionInStream.
    + * This will be overridden by more specific types.
    + */
    +sealed trait InitialPosition {
    +  val initialPositionInStream: InitialPositionInStream
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.LATEST.
    + */
    +case object Latest extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.LATEST
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.TRIM_HORIZON.
    + */
    +case object TrimHorizon extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.TRIM_HORIZON
    +}
    +
    +/**
    + * Case object for Kinesis's InitialPositionInStream.AT_TIMESTAMP.
    + */
    +case class AtTimestamp(timestamp: Date) extends InitialPosition {
    +  val instance: InitialPosition = this
    +  override val initialPositionInStream: InitialPositionInStream
    +    = InitialPositionInStream.AT_TIMESTAMP
    +}
    +
    +/**
    + * Companion object for InitialPosition that returns
    + * appropriate version of InitialPositionInStream.
    + */
    +object InitialPosition {
    --- End diff --
    
    I like `InitialPosition initialPosition = KinesisInitialPosition.TrimHorizon();` best :)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77185/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    **[Test build #77700 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77700/testReport)** for PR 18029 at commit [`3cac39b`](https://github.com/apache/spark/commit/3cac39b49b7b56547eee117f53d0182798270111).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by budde <gi...@git.apache.org>.
Github user budde commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @yssharma Apologies for the slow reply-- very little of my professional dev work happens on GitHub so I can be prone to miss things :-/
    
    You would introduce a ```InitialPosition``` interface with three implementations, as @brkyvz has suggested. ```builder. initialPositionInStream()```  would be replaced by a method (perhaps of the same name or just ```initialPosition()```) that takes ```InitialPosition``` as an input argument. Users can then provide whatever ```InitialPosition``` implementation is appropriate for them. The existing default value can be used if one is not specified. This method shouldn't need to be overloaded.
    
    Users could either construct their ```InitialPosition``` implementation directly or we could introduce a builder interface to do this as we did with [SparkAWSCredentials](https://github.com/apache/spark/blob/38f4e8692ce3b6cbcfe0c1aff9b5e662f7a308b7/external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/SparkAWSCredentials.scala). The ```SparkAWSCredentials``` interface is actually a good mode to follow here, I think.
    
    I'm not a committer to Spark but I'd strongly be in favor of consolidating the initial position options under a single interface rather than introducing redundant arguments that we'll just want to replace later anyhow.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79384/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Will wait for @brkyvz , @HyukjinKwon for final ☑️ 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18029: [SPARK-20168][WIP][DStream] Add changes to use ki...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18029#discussion_r120236128
  
    --- Diff: external/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisInputDStream.scala ---
    @@ -193,6 +197,21 @@ object KinesisInputDStream {
         }
     
         /**
    +     * Sets the Kinesis initial position data to the provided timestamp.
    +     * Sets InitialPositionInStream to [[InitialPositionInStream.AT_TIMESTAMP]]
    +     * and the timestamp to the provided value.
    +     *
    +     * @param timestamp Timestamp to resume the Kinesis stream from a provided
    +     *                  timestamp.
    +     * @return Reference to this [[KinesisInputDStream.Builder]]
    +     */
    +    def withTimestampAtInitialPositionInStream(timestamp: Date) : Builder = {
    --- End diff --
    
    Got it now. Read your new comment.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168][WIP][DStream] Add changes to use kinesis f...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    @budde Added a new method to pass the timestamp more elegantly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18029: [SPARK-20168] [DStream] Add changes to use kinesis fetch...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/18029
  
    Hi @brkyvz can I get some love here please.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org