Skip to content

[Fix][Connector-V2] Add missing MULTI_TABLE_SINK_REPLICA option to PulsarSinkFactory#11160

Open
nzw921rx wants to merge 3 commits into
apache:devfrom
nzw921rx:fix/pulsar-sink-multi-table-replica-option
Open

[Fix][Connector-V2] Add missing MULTI_TABLE_SINK_REPLICA option to PulsarSinkFactory#11160
nzw921rx wants to merge 3 commits into
apache:devfrom
nzw921rx:fix/pulsar-sink-multi-table-replica-option

Conversation

@nzw921rx

Copy link
Copy Markdown
Collaborator

Purpose of this pull request

PulsarSinkFactory implements SupportMultiTableSinkWriter but its optionRule() did not declare SinkConnectorCommonOptions.MULTI_TABLE_SINK_REPLICA as an optional parameter, causing ConnectorSpecificationCheckTest to fail:

org.opentest4j.AssertionFailedError: Please add `SinkCommonOptions.MULTI_TABLE_SINK_REPLICA` optional into the `optionRule` method optional of `PulsarSinkFactory` ==> expected: <true> but was: <false>
	at org.apache.seatunnel.api.connector.ConnectorSpecificationCheckTest.checkSupportMultiTableSink(ConnectorSpecificationCheckTest.java:185)
	at org.apache.seatunnel.api.connector.ConnectorSpecificationCheckTest.testAllConnectorImplementFactoryWithUpToDateMethod(ConnectorSpecificationCheckTest.java:172)

Changes

  • Added SinkConnectorCommonOptions.MULTI_TABLE_SINK_REPLICA to the .optional(...) list in PulsarSinkFactory.optionRule()

How was this patch tested?

./mvnw test -pl seatunnel-dist -Dtest=ConnectorSpecificationCheckTest#testAllConnectorImplementFactoryWithUpToDateMethod

@DanielLeens DanielLeens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the quick follow-up here. I re-checked the current head against the multi-table sink contract and I do not see a source-level blocker.

What this PR fixes

  • User pain: The Pulsar sink already implements the multi-table sink contract, but the factory was missing MULTI_TABLE_SINK_REPLICA in its option rule, so connector-spec validation fails before users can reach the runtime path.
  • Fix approach: The PR adds SinkConnectorCommonOptions.MULTI_TABLE_SINK_REPLICA back to PulsarSinkFactory.optionRule().
  • One-line summary: This is a precise contract-alignment fix between the factory option rule and the already implemented multi-table sink runtime path.

Runtime path I traced

Job submission
  -> `PulsarSinkFactory.optionRule()` now declares `MULTI_TABLE_SINK_REPLICA`

Runtime contract
  -> `PulsarSink` implements `SupportMultiTableSink`
  -> `createWriter()` returns `PulsarSinkWriter`
  -> `PulsarSinkWriter` implements `SupportMultiTableSinkWriter<Void>`

Result
  -> the submission-time contract now matches the existing multi-table runtime path

Key findings

  • The normal connector-spec / submission path hits this change directly.
  • The fix is precise: it exposes an already implemented runtime capability instead of changing write behavior.
  • I re-checked the factory, sink, and writer together, and the contract is now complete.

Blocking items
None at the source-code level on the current head.

Other review notes

  • No additional non-blocking source-level comments from this round.

Compatibility and side effects

  • Compatibility: Fully backward compatible. This only restores a missing option declaration.
  • Side effects: No runtime write-path or resource-model side effects from this change.
  • Error handling / logging: No new error-handling or logging concerns from this diff.
  • Tests: For this contract fix, the key proof is the factory/sink/writer contract alignment, which now looks complete on the current head.
  • Docs: No docs update is required.
  • CI: Some checks were still queued, but that does not change the source-level conclusion.

Merge verdict

  • From the current head, I do not see a source-level blocker. This looks good to merge.

@zhangshenghang

Copy link
Copy Markdown
Member

LGTM if the CI passes,You can try again

@github-actions github-actions Bot added the e2e label Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants