Skip to content

Conversation

@junyuc25
Copy link
Contributor

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

junyuche added 4 commits July 3, 2023 11:16
Update source files

Update test files

Fix initialization errors during tests

Remove RegionUtil usages and fix Guava NoSuchMethodError

Update guava version

Update KinesisUtilsPythonHelper

Upgrade AWS SDK to v2 in kubernetes test module

Remove tags

Update kinesis docs

Remove sdk dependency in core module

Remove unused import
<!-- Should be consistent with Kinesis client dependency -->
<aws.java.sdk.version>1.11.655</aws.java.sdk.version>
<aws.java.sdk.version>1.12.447</aws.java.sdk.version>
<aws.java.sdk.v2.version>2.17.268</aws.java.sdk.v2.version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this PR aiming to upgrade both V1 and V2 at the same time?

@pan3793
Copy link
Member

pan3793 commented Aug 21, 2023

Is it possible to provide dedicated credentials for the Spark community to run CI with AWS services?


addSbtPlugin("com.thesamet" % "sbt-protoc" % "1.0.6")

addDependencyTreePlugin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will eventually be revert, right?

@LuciferYang
Copy link
Contributor

LuciferYang commented Aug 21, 2023

I have two concerns about this pr

  1. Module streaming-kinesis-asl has 60 UTs, but 24 are ignored due to the environment variable ENABLE_KINESIS_TESTS defaulting to 0.
[info] Tests: succeeded 34, failed 0, canceled 0, ignored 24, pending 0
[info] All tests passed.

However, when I run ENABLE_KINESIS_TESTS=1 build/sbt "streaming-kinesis-asl/test" -Pkinesis-asl, these previously ignored test cases fail.

[info] *** 4 SUITES ABORTED ***
[error] Error: Total 36, Failed 0, Errors 4, Passed 32
[error] Error during tests:
[error] 	org.apache.spark.streaming.kinesis.WithoutAggregationKinesisBackedBlockRDDSuite
[error] 	org.apache.spark.streaming.kinesis.WithAggregationKinesisBackedBlockRDDSuite
[error] 	org.apache.spark.streaming.kinesis.WithAggregationKinesisStreamSuite
[error] 	org.apache.spark.streaming.kinesis.WithoutAggregationKinesisStreamSuite
[error] (streaming-kinesis-asl / Test / test) sbt.TestsFailedException: Tests unsuccessful

It seems we also can't manually verify this module without credentials?

  1. The CI does not provide sufficient check for this module

# TODO(SPARK-32246): We don't test 'streaming-kinesis-asl' for now.

So, I think we should complete the TODO SPARK-32246 before proceeding with this update.

@github-actions
Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Nov 30, 2023
@github-actions github-actions bot closed this Dec 1, 2023
dongjoon-hyun pushed a commit that referenced this pull request Dec 4, 2023
…ithub Action CI

### What changes were proposed in this pull request?
This PR attempts to set up Kinesis tests in one of the existing Github Actions. Note that currently there are totally 57 tests in the Kinesis-asl module, and this PR enabled 35 of them. The remaining tests requires interaction with Amazon Kinesis service which would incur billing costs to users. Hence they are not included in the Github Action.

### Why are the changes needed?

Addressing the comments in this PR: #42581 (comment) which attempts to upgrade the AWS SDK to v2 for Spark Kinesis connector. Since Kinesis tests are not being run in the Github Actions, there is no automated mechanism to verify the SDK v2 upgrade changes in this module.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

1. All existing Github Actions passed.
2. All Kinesis tests passed when running locally: `export ENABLE_KINESIS_TESTS=1 && mvn test -Pkinesis-asl -pl connector/kinesis-asl`
```
Tests: succeeded 57, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  13:25 min
[INFO] Finished at: 2023-11-12T00:15:49+08:00
[INFO] ------------------------------------------------------------------------
```

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #43736 from junyuc25/junyuc25/kinesis-test.

Authored-by: Junyu Chen <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
asl3 pushed a commit to asl3/spark that referenced this pull request Dec 5, 2023
…ithub Action CI

### What changes were proposed in this pull request?
This PR attempts to set up Kinesis tests in one of the existing Github Actions. Note that currently there are totally 57 tests in the Kinesis-asl module, and this PR enabled 35 of them. The remaining tests requires interaction with Amazon Kinesis service which would incur billing costs to users. Hence they are not included in the Github Action.

### Why are the changes needed?

Addressing the comments in this PR: apache#42581 (comment) which attempts to upgrade the AWS SDK to v2 for Spark Kinesis connector. Since Kinesis tests are not being run in the Github Actions, there is no automated mechanism to verify the SDK v2 upgrade changes in this module.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

1. All existing Github Actions passed.
2. All Kinesis tests passed when running locally: `export ENABLE_KINESIS_TESTS=1 && mvn test -Pkinesis-asl -pl connector/kinesis-asl`
```
Tests: succeeded 57, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  13:25 min
[INFO] Finished at: 2023-11-12T00:15:49+08:00
[INFO] ------------------------------------------------------------------------
```

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#43736 from junyuc25/junyuc25/kinesis-test.

Authored-by: Junyu Chen <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
dbatomic pushed a commit to dbatomic/spark that referenced this pull request Dec 11, 2023
…ithub Action CI

### What changes were proposed in this pull request?
This PR attempts to set up Kinesis tests in one of the existing Github Actions. Note that currently there are totally 57 tests in the Kinesis-asl module, and this PR enabled 35 of them. The remaining tests requires interaction with Amazon Kinesis service which would incur billing costs to users. Hence they are not included in the Github Action.

### Why are the changes needed?

Addressing the comments in this PR: apache#42581 (comment) which attempts to upgrade the AWS SDK to v2 for Spark Kinesis connector. Since Kinesis tests are not being run in the Github Actions, there is no automated mechanism to verify the SDK v2 upgrade changes in this module.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

1. All existing Github Actions passed.
2. All Kinesis tests passed when running locally: `export ENABLE_KINESIS_TESTS=1 && mvn test -Pkinesis-asl -pl connector/kinesis-asl`
```
Tests: succeeded 57, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  13:25 min
[INFO] Finished at: 2023-11-12T00:15:49+08:00
[INFO] ------------------------------------------------------------------------
```

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#43736 from junyuc25/junyuc25/kinesis-test.

Authored-by: Junyu Chen <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants