Skip to content

DBZ#1668 Add descriptors generation to Debezium Server sinks#257

Merged
mfvitale merged 8 commits intodebezium:mainfrom
mfvitale:dbz#1668
Apr 15, 2026
Merged

DBZ#1668 Add descriptors generation to Debezium Server sinks#257
mfvitale merged 8 commits intodebezium:mainfrom
mfvitale:dbz#1668

Conversation

@mfvitale
Copy link
Copy Markdown
Member

@mfvitale mfvitale commented Mar 16, 2026

@mfvitale
Copy link
Copy Markdown
Member Author

mfvitale commented Mar 16, 2026

Just a draft to see if we like going in this direction.

I would like to mainly receive feedback on the DebeziumServerSink interface. The idea is to have a common interface to guide a bit what needs to be implemented for a sink.

The idea is also to easily share common code like validation and configurations of Debezium Server sink so that them can be just reused elsewhere (i.e Debezium Platform).

@jpechane @Naros @kmos @rk3rn3r I really appreciate your feedback on this.

@jpechane
Copy link
Copy Markdown
Contributor

@mfvitale I wonder if DebeziumServerSink should extend DebeziumEngine.ChangeConsumer.
Also IIUIC we are using only proprty discovery and loading from MP Config and completely ignore the validation.

@mfvitale
Copy link
Copy Markdown
Member Author

@mfvitale I wonder if DebeziumServerSink should extend DebeziumEngine.ChangeConsumer.

My main idea here is to have a single DebeziumEngine.ChangeConsumer as now we have DefaultChangeConsumer that will manage the lifecycle of the sinks calling the right method of that interface.

Also IIUIC we are using only proprty discovery and loading from MP Config and completely ignore the validation.

We can also think to add the validation.

So thinking also to the upcoming migration to quarkus-extension in the first phase we could have just the DefaultChangeConsumer rewritten in the new form and then in the future we can think on an intermediate layer also for sinks (another extension ?)

I will also let @kmos add his bits in this regards.

In any case the part relevant for the scope of this PR is the ones related to component descriptors

public interface DebeziumServerSink extends ConfigDescriptor, ComponentMetadataProvider

the rest could be just a draft and discussed in a dedicated issue, just to avoid blocking the component descriptors for sinks.

@kmos
Copy link
Copy Markdown
Member

kmos commented Mar 20, 2026

So thinking also to the upcoming migration to quarkus-extension in the first phase we could have just the DefaultChangeConsumer rewritten in the new form and then in the future we can think on an intermediate layer also for sinks (another extension ?)

Exactly. As @mfvitale mentioned, in the first iteration we can migrate to QDE by capturing the DefaultChangeConsumer, which should align with his proposal. In a subsequent iteration, we can make the consumer independent by using the QDE API directly and start the initiative for a CDI‑ready Debezium Server sink. This would also allow us to incorporate descriptor validation, similar to exposing the getConfigFields() API through annotations.

@mfvitale
Copy link
Copy Markdown
Member Author

mfvitale commented Mar 30, 2026

Failure addressed with #262

@jpechane @Naros Could you take a final look to? I would just add @Incubating to the new interface since this is something we will still work on during the DS migration to quarkus-extension.

Ideally I would like to have the changes to the consumers merged before the 3.5.0.Final to have the descriptors in place.

@mfvitale mfvitale marked this pull request as ready for review March 31, 2026 14:43
@mfvitale mfvitale changed the title Dbz#1668 Add descriptors generation to Debezium Server sinks DBZ#1668 Add descriptors generation to Debezium Server sinks Apr 1, 2026
@mfvitale mfvitale marked this pull request as draft April 1, 2026 15:08
@mfvitale mfvitale marked this pull request as ready for review April 2, 2026 09:51
@mfvitale mfvitale requested review from Naros and vjuranek April 2, 2026 10:59
Copy link
Copy Markdown
Member

@Naros Naros left a comment

Choose a reason for hiding this comment

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

Hi @mfvitale I left just a few inline questions.

Comment on lines +89 to +90
configuredPartitionId = (config.getConfiguredPartitionId() != null) ? config.getConfiguredPartitionId() : "";
configuredPartitionKey = (config.getConfiguredPartitionKey() != null) ? config.getConfiguredPartitionKey() : "";
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.

Could we use Strings.defaultIfEmpty for these types of cases?


final String serverHost = config.getServerHost();
final String cacheName = config.getCacheName();
final Integer serverPort = config.getServerPort() != null ? config.getServerPort() : ConfigurationProperties.DEFAULT_HOTROD_PORT;
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.

For situations like this, would it make sense to abstract this detail away inside the various sink-specific configuration class so for this call site it would simply be config.getServerPort() ? We could probably extend this to a variety of other places and minimize redundancy.

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.

Or a helper method so it will be like Util.defaultWhenNull(config.getServerPort(),ConfigurationProperties.DEFAULT_HOTROD_PORT)

Copy link
Copy Markdown
Member Author

@mfvitale mfvitale Apr 14, 2026

Choose a reason for hiding this comment

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

Well since we use the io.debezium.config.Configuration inside the configuration classes we can use it to get the defaults and or properly set default on Field

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.

@mfvitale Good point! This should be added in all places but I am fine doing that kind of cleanup in a separate PR.

Copy link
Copy Markdown
Member

@kmos kmos left a comment

Choose a reason for hiding this comment

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

Apart from what we discussed offline, my only suggestion is (if possible) to move the interfaces into another package, such as *.api or something similar.

Signed-off-by: Fiore Mario Vitale <mvitale@redhat.com>
Signed-off-by: Fiore Mario Vitale <mvitale@redhat.com>
Signed-off-by: Fiore Mario Vitale <mvitale@redhat.com>
Signed-off-by: Fiore Mario Vitale <mvitale@redhat.com>
Signed-off-by: Fiore Mario Vitale <mvitale@redhat.com>
Signed-off-by: Fiore Mario Vitale <mvitale@redhat.com>
Signed-off-by: Fiore Mario Vitale <mvitale@redhat.com>
Signed-off-by: Fiore Mario Vitale <mvitale@redhat.com>
@mfvitale
Copy link
Copy Markdown
Member Author

@Naros @jpechane Could you take another pass?

@mfvitale mfvitale merged commit 984f09b into debezium:main Apr 15, 2026
7 of 8 checks passed
@mfvitale mfvitale deleted the dbz#1668 branch April 15, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add descriptors generation to Debezium Server sinks

4 participants