Skip to content
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

KAFKA-15387: Deprecate Connect's redundant task configurations endpoint #14361

Merged
merged 2 commits into from
Oct 14, 2023

Conversation

yashmayya
Copy link
Contributor

Screenshot 2023-09-08 at 11 28 11 PM

Committer Checklist (excluded from commit message)

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

@yashmayya yashmayya added connect kip Requires or implements a KIP labels Sep 8, 2023
@yashmayya yashmayya requested review from C0urante, mimaison and gharris1727 and removed request for C0urante September 8, 2023 18:02
Copy link
Collaborator

@vamossagar12 vamossagar12 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 for the fix Yash! I would have preferred a test which would have invoked the older end point and validates that the warning log line gets printed though. Other than that, seems fine.

@yashmayya
Copy link
Contributor Author

Thanks Sagar!

a test which would have invoked the older end point and validates that the warning log line gets printed

Feels like a bit of overkill, but I guess it doesn't really hurt to add such a test so I've added one. There's currently no integration test class for Connect's REST API - most of the functionality is tested indirectly through other ITs and some of the more specialized ones have their own test classes (ConnectorRestartApiIntegrationTest, ConnectorTopicsIntegrationTest, OffsetsApiIntegrationTest etc.). It didn't seem worth it to introduce a new integration test class just for this small test so I've added it to ConnectWorkerIntegrationTest which is the most generic one.

@yashmayya yashmayya force-pushed the KAFKA-15387-deprecate-tasks-config branch from 09cdc5f to 00b73c3 Compare September 11, 2023 12:47
Copy link
Member

@mimaison mimaison left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. LGTM

@yashmayya
Copy link
Contributor Author

Test failures are unrelated, merging to trunk

@yashmayya yashmayya merged commit 1c8bb61 into apache:trunk Oct 14, 2023
1 check failed
AnatolyPopov pushed a commit to aiven/kafka that referenced this pull request Feb 16, 2024
…nt (apache#14361)

Reviewers: Mickael Maison <mickael.maison@gmail.com>, Sagar Rao <sagarmeansocean@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connect kip Requires or implements a KIP
Projects
None yet
3 participants