-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[BEAM-8719 BEAM-8768 BEAM-8769 BEAM-8770 BEAM-8771] Update minor hadoop dependencies #13230
Conversation
240484f
to
41300df
Compare
Run Java PostCommit |
Run SQL PostCommit |
Run Spark ValidatesRunner |
Run Java HadoopFormatIO Performance Test |
@iemejia @timrobertson100 @aromanenko-dev Is this approach of testing the previus version of Hadoop acceptable? Could I ask you all to take a look at this PR? |
outputs.upToDateWhen { false } | ||
include '**/*Test.class' | ||
include '**/HadoopFormatIOElasticIT.class' |
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.
Without looking in detail, this looks like it might be worth checking. Seems to set up for Cassandra, but runs ES tests?
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.
Oh right, it makes no sense to run HadoopFormatIOElasticIT
here. This test has a default configuration that can be run without setting up anything but it indeed is inconsistent with the whole idea of hadoopVersion285Test
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.
Cassandra is set for HadoopFormatIOCassandraTest
I think this makes sense to add. Thanks @piotr-szuberski In our case, the Hadoop version is so tightly coupled with the Spark, Hive etc versions (managed Cloudera cluster) that it's probably insufficient to test things in isolation - it's a solid addition regardless. |
Yeah, I guess. I added just the precommit test suites. |
By the way, could we use the same approach to test KafkaIO against different versions? It's already done for 2.1.0 - does anything stop us from doing the same thing there? |
@piotr-szuberski I think it's quite reasonable to test different gradle tasks to test with different Kafka client versions. As additional bonus, it would be great to run ITs against different versions of Kafka docker images as well. |
Okay, I'll try to do it next week! |
64dc537
to
4fc1370
Compare
@timrobertson100 Should I change something in this PR? I think the IT tests could be done in a separate PR - WDYT? |
I agree. Other than squashing into a single commit I don't think you need to change anything here |
4fc1370
to
f9492fb
Compare
Run Java PostCommit |
Run SQL PostCommit |
Run Java HadoopFormatIO Performance Test |
Run Spark ValidatesRunner |
Run Java PreCommit |
@timrobertson100 Ok, it's squashed now. |
Thanks @piotr-szuberski Waiting for green, then it can be merged |
@timrobertson100 green :) |
Thanks @piotr-szuberski ! |
No problem! I'll soon publish IT tests on Java postcommit and tests with Hadoop 3.2.1 |
This PR:
For the tests I've taken the approach that is already used in KafkaIO.
I tried to add tests for Hadoop 2.7.7 but without success. I haven't verified if the problem comes from tests configuration or the IO itself but since this PR is to assure the compatibility with the previous version used in Beam (2.8.5) I'd like not to touch it. It could be worth to file a Jira to add tests for 2.7.7 version.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username
).[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.