Skip to content

[CDAP-17402] Upgrade debizum version to 1.3.1.Final#109

Merged
sagarkapare merged 5 commits intodevelopfrom
bugfix_release/upgrade-debezium-version
Dec 14, 2020
Merged

[CDAP-17402] Upgrade debizum version to 1.3.1.Final#109
sagarkapare merged 5 commits intodevelopfrom
bugfix_release/upgrade-debezium-version

Conversation

@sagarkapare
Copy link
Copy Markdown
Contributor

Upgrade the debezium version to 1.3.1.Final. Currently we use 0.9.5.Final which is outdated.
This PR mainly copies and fixes checkstyle issues in the debezium classes: MySQLConnectorConfig, MySqlJdbcContext, MySqlValueConverters, and SqlServerConnections corresponding to version 1.3.1.Final. This was done for 0.9.5.Final version in these PRs - #35 and #38.

}

jdbcConfig = jdbcConfigBuilder.build();
String driverClassName = jdbcConfig.getString(MySqlConnectorConfig.JDBC_DRIVER);
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.

looks like the local variable driverClassName is not referenced in this method

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes thats correct. driverClassName is used in the original file - https://github.com/debezium/debezium/blob/v1.3.1.Final/debezium-connector-mysql/src/main/java/io/debezium/connector/mysql/MySqlJdbcContext.java#L84 however we pass in the connectionFactory that we set. To keep the changes in these copied file minimum, I kept this variable as it is.

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 information :)

.filter(x -> !(x.startsWith(DatabaseHistory.CONFIGURATION_FIELD_PREFIX_STRING) ||
x.equals(MySqlConnectorConfig.DATABASE_HISTORY.name())))
.edit()
.withDefault(MySqlConnectorConfig.PORT, MySqlConnectorConfig.PORT.defaultValue())
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.

do we need this line ? If the Field already has a "defaultValue" method , do we need to override the default value here this way ?

I might be wrong since I'm not familiar with Debezium API. Looks like this method is for overriding the default value of a field. Otherwise it just need one argument Field

public EventProcessingFailureHandlingMode eventProcessingFailureHandlingMode() {
String mode = config.getString(CommonConnectorConfig.EVENT_PROCESSING_FAILURE_HANDLING_MODE);
if (mode == null) {
mode = config.getString(MySqlConnectorConfig.EVENT_DESERIALIZATION_FAILURE_HANDLING_MODE);
Copy link
Copy Markdown
Contributor

@seanzhougoogle seanzhougoogle Dec 14, 2020

Choose a reason for hiding this comment

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

so this can't be null ? Do we need to handle the error if this one is null too ?

}

/**
* Determine the difference between two sets.
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.

nit : "between two GTID sets"

Copy link
Copy Markdown
Contributor

@seanzhougoogle seanzhougoogle left a comment

Choose a reason for hiding this comment

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

Thank you sagar:)

@sagarkapare
Copy link
Copy Markdown
Contributor Author

Thanks Sean for the review. As discussed keeping the classes from debezium as it is except where we absolutely need to change them.

@sagarkapare sagarkapare merged commit 3e75f43 into develop Dec 14, 2020
@sagarkapare sagarkapare deleted the bugfix_release/upgrade-debezium-version branch December 14, 2020 23:37
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.

2 participants