Skip to content

MINOR: Add more versions to connect compatibility#7024

Merged
cmccabe merged 1 commit intoapache:trunkfrom
rhauch:add-more-versions-to-connect-compatibility
Sep 18, 2019
Merged

MINOR: Add more versions to connect compatibility#7024
cmccabe merged 1 commit intoapache:trunkfrom
rhauch:add-more-versions-to-connect-compatibility

Conversation

@rhauch
Copy link
Copy Markdown
Contributor

@rhauch rhauch commented Jul 2, 2019

ConnectDistributedTest.test_broker_compatibility hasn't been updated with recent Kafka versions in quite some time.

This can be merged to trunk and easily backported to 2.3, but if backported farther back some versions would have to be removed for each branch. For example, the 2.2 branch should not have a compatibility test for 2.3, and the 2.1 branch should not have a compatibility test for 2.3 or 2.2.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@rhauch rhauch force-pushed the add-more-versions-to-connect-compatibility branch from a4ab685 to 84b7f74 Compare July 2, 2019 02:30
@rhauch
Copy link
Copy Markdown
Contributor Author

rhauch commented Jul 2, 2019

See https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2762/ for system test run with just the affected system test method.

Copy link
Copy Markdown
Contributor

@wicknicks wicknicks left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @rhauch!

@rhauch
Copy link
Copy Markdown
Contributor Author

rhauch commented Jul 2, 2019

Actually, this is currently dependent upon #7023, but #7025 is an alternative to #7023. If #7025 is chosen, then this probably needs to be updated to remove the first commit.

@rhauch rhauch force-pushed the add-more-versions-to-connect-compatibility branch from 84b7f74 to 7028edf Compare July 12, 2019 16:21
@rhauch rhauch force-pushed the add-more-versions-to-connect-compatibility branch from 7028edf to e0fa759 Compare July 12, 2019 16:25
@rhauch
Copy link
Copy Markdown
Contributor Author

rhauch commented Jul 12, 2019

Rebased on trunk since #7023 was merged (and #7025 was not).

@kkonstantine, would you mind reviewing to make sure these combinations are acceptable?

Copy link
Copy Markdown
Contributor

@kkonstantine kkonstantine left a comment

Choose a reason for hiding this comment

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

Good to have increased coverage.
LGTM!

@rhauch
Copy link
Copy Markdown
Contributor Author

rhauch commented Jul 24, 2019

Started another test run of just this parameterized test so we can measure how long it takes to run the tests: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2817/

@rhauch
Copy link
Copy Markdown
Contributor Author

rhauch commented Jul 24, 2019

This change results in 22 different test variations for this one method, and the last branch builder run took ~17 minutes to run all 22 tests. That is not a significant increase in time, so IMO this extra validation is worth the minimal cost.

@rajinisivaram or @hachikuji, could one of you review and merge back to the 2.3 branch?

@rhauch
Copy link
Copy Markdown
Contributor Author

rhauch commented Aug 13, 2019

@rajinisivaram or @hachikuji: ping again for review and merge back to the 2.3 branch.

@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Sep 18, 2019

LGTM

@cmccabe cmccabe merged commit ada35d5 into apache:trunk Sep 18, 2019
asfgit pushed a commit that referenced this pull request Sep 18, 2019
…7024)

Reviewers: Arjun Satish <arjun@confluent.io>, Konstantine Karantasis <k.karantasis@gmail.com>
(cherry picked from commit ada35d5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants