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

DBZ-3141 Support for signalling table #2182

Merged
merged 8 commits into from Mar 10, 2021
Merged

Conversation

jpechane
Copy link
Contributor

@jpechane jpechane commented Mar 4, 2021

@jpechane jpechane marked this pull request as ready for review March 4, 2021 10:20
@jpechane jpechane force-pushed the DBZ-3141 branch 2 times, most recently from ace44fe to 3203a71 Compare March 5, 2021 12:51
@jpechane
Copy link
Contributor Author

jpechane commented Mar 5, 2021

Oracle UX can be improved via debezium/debezium-examples#161

@jpechane jpechane force-pushed the DBZ-3141 branch 3 times, most recently from 58677a8 to 1f07871 Compare March 8, 2021 13:11
@jpechane
Copy link
Contributor Author

jpechane commented Mar 8, 2021

Copy link
Member

@gunnarmorling gunnarmorling left a comment

Choose a reason for hiding this comment

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

@jpechane, a few comments inline. So no usage of that for Oracle DDL changes yet, only the basic infra, right?

.withType(Type.STRING)
.withWidth(Width.MEDIUM)
.withImportance(Importance.MEDIUM)
.withDescription("The name of the data collection that is used to send signlas/commands to Debezium. Signaling is disabled when not set.");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.withDescription("The name of the data collection that is used to send signlas/commands to Debezium. Signaling is disabled when not set.");
.withDescription("The name of the data collection that is used to send signals/commands to Debezium. Signaling is disabled when not set.");

private final CommonConnectorConfig connectorConfig;
private final String signalDataCollectionId;

private final Map<String, Action> signalActions = new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Is this accessed concurrently? Let's add some @ThreadSafe etc. annotations.

@gunnarmorling
Copy link
Member

gunnarmorling commented Mar 9, 2021 via email

@jpechane
Copy link
Contributor Author

jpechane commented Mar 9, 2021

@gunnarmorling
Copy link
Member

gunnarmorling commented Mar 9, 2021 via email

@gunnarmorling
Copy link
Member

@jpechane, ok, so what's the purpose of using the signal table for Postgres?

@jpechane
Copy link
Contributor Author

jpechane commented Mar 9, 2021

@gunnarmorling The prupose is twofold

  • have a regularly running IT test
  • solve the issue with PK definition change

@gunnarmorling
Copy link
Member

Ah, gotcha. So we amend the schema evolution story for PG, too. Ok, can you then amend the documentation accordingly, too?

@jpechane
Copy link
Contributor Author

jpechane commented Mar 9, 2021

@gunnarmorling Could you please merge the PR and I'll post the docs in a separate one so we can get tests passing again?
The ramining failures are fixed via #2200

@gunnarmorling
Copy link
Member

@jpechane, are you still planning to push a commit with doc updates for this one?

Copy link
Member

@gunnarmorling gunnarmorling left a comment

Choose a reason for hiding this comment

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

LGTM, great work. I'll apply that one JavaDoc fix and merge. Thanks a lot, @jpechane!

import io.debezium.util.Testing;

/**
* Integration test to check transaction metadata.
Copy link
Member

Choose a reason for hiding this comment

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

Copy/paste glitch?

TestHelper.execute(INSERT_STMT);

// Insert the signal record - add 'aa' column to PK fields
TestHelper.execute("INSERT INTO s1.debezium_signal VALUES('1', 'schema-changes', '{\"database\": \"postgres\", \"changes\": [{\n"
Copy link
Member

Choose a reason for hiding this comment

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

I wish we'd use Java 13+ and its test blocks for tests...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants