Skip to content

MINOR: Convert connect assignment schemas to use generated protocol#9641

Closed
chia7712 wants to merge 13 commits intoapache:trunkfrom
chia7712:MINOR-9641
Closed

MINOR: Convert connect assignment schemas to use generated protocol#9641
chia7712 wants to merge 13 commits intoapache:trunkfrom
chia7712:MINOR-9641

Conversation

@chia7712
Copy link
Copy Markdown
Member

as title.

Committer Checklist (excluded from commit message)

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

public static final String ASSIGNMENT_KEY_NAME = "assignment";
public static final int CONNECTOR_TASK = -1;

public static final short CONNECT_PROTOCOL_V0 = 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The CONNECT_PROTOCOL_V0 is only used in ConnectProtocolCompatibility.fromProtocolVersion(), I think we shouldn't depend on the version to compute ConnectProtocolCompatibility because every time we bump the version we should change the method, maybe we can add a field in the ExtendedWorkerMetadata.json to represent the corresponding ConnectProtocolCompatibility.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I feel the version is bump only if we add new protocol (see https://github.com/apache/kafka/blob/trunk/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/WorkerCoordinator.java#L190).

For another, your comment inspires me that the constant field can be moved to ConnectProtocolCompatibility as it is used by ConnectProtocolCompatibility only

Comment thread connect/runtime/src/main/resources/common/message/ConnectAssignment.json Outdated
@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 25, 2021

cc @kkonstantine

@ijuma ijuma requested a review from kkonstantine January 25, 2021 11:10
Comment thread connect/runtime/src/main/resources/common/message/ConnectAssignment.json Outdated
@chia7712 chia7712 marked this pull request as draft January 26, 2021 09:12
@chia7712
Copy link
Copy Markdown
Member Author

convert this patch to draft as some system tests can't pass on my local

@chia7712 chia7712 marked this pull request as ready for review January 26, 2021 14:57
@chia7712 chia7712 closed this Mar 25, 2024
@chia7712 chia7712 deleted the MINOR-9641 branch March 25, 2024 15:21
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