-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-25111][BUILD] increment kinesis client/producer & aws-sdk versions #22099
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-25111][BUILD] increment kinesis client/producer & aws-sdk versions #22099
Conversation
…k to match. Change-Id: Ic2d12a07d273bd1b6fc4c681075070f22ed1e44c
|
As noted in #22146; stripping off bouncy castle and upgrading the SDK worked. But a local test run of just this patch brought up the same error seen in #22081 That wasn't a full clean build, so let's see what Jenkins says and some more test runs tomorrow. It could just be this is all showing up some flakiness in the test case. At the very least, some more details on the failure might be good. |
|
Test build #94728 has finished for PR 22099 at commit
|
srowen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK in principle, pending tests.
|
Test build #4246 has finished for PR 22099 at commit
|
|
Test build #4247 has started for PR 22099 at commit |
|
Local kinesis tests with both -Phadoop-3.1, -Phadoop-2.7 & I'm updating the #21146 PR with this patch to see what happens with the combination in Jenkins of no bouncycastle, updated Kinesis. Test run failure here was |
|
Retest this please. |
|
Test build #94776 has finished for PR 22099 at commit
|
|
To be clear you think this passed because it still uses jets3t and that still brings in BC? Then we can maybe merge this and rebase the other change to find out. This update won't have changed that situation with strong crypto being required right? |
don't know. What it did do was stop my local test runs without bouncy castle failing with errors about certificate validation. This patch is a good thing to do anyway, because it's good to stay somewhat current with the AWS releases (more chance of issues being addressed, reduced cost of future migrations). So it can be merged in and then the problem of getting #22081's test run to work addressed after. I reopened #21146 & applied this patched to it, to see what Jenkins did there. The overall test runs come out as failing -hard to point to any related cause, but the Kinesis ones do all pass: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94769/testReport/org.apache.spark.streaming.kinesis/ I'm going to close that one again to avoid confusion about which of the "remove jets3t" patches people should be looking at; once the kinesis update is merged in you'll need to retest your #22081 PR and let's see what Jenkins says there |
|
Merged to master |
|
thanks |
This PR has been superceded by #22081
What changes were proposed in this pull request?
Increment the kinesis client, producer and transient AWS SDK versions to a more recent release.
This is to help with the move off bouncy castle of #21146 and #22081; the goal is that moving up to the new SDK will allow a JVM with unlimited JCE but without bouncy castle to work with Kinesis endpoints.
Why this specific set of artifacts? it syncs up with the 1.11.271 AWS SDK used by hadoop 3.0.3, hadoop-3.1. and hadoop 3.1.1; that's been stable for the uses there (s3, STS, dynamo).
How was this patch tested?
Running all the external/kinesis-asl tests via maven with java 8.121 & unlimited JCE, without bouncy castle (#21146); default endpoint of us-west.2. Without this SDK update I was getting http cert validation errors, with it they went away.
This PR is not ready without