You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/06/23 14:47:28 UTC
[GitHub] [kafka] rhauch commented on a change in pull request #10854: KAFKA-12717: Remove internal Connect converter properties (KIP-738)
rhauch commented on a change in pull request #10854:
URL: https://github.com/apache/kafka/pull/10854#discussion_r657156175
##########
File path: docs/upgrade.html
##########
@@ -76,6 +76,12 @@ <h5><a id="upgrade_300_notable" href="#upgrade_300_notable">Notable changes in 3
understood by brokers or version 2.5 or higher, so you must upgrade your kafka cluster to get the stronger semantics. Otherwise, you can just pass
in <code>new ConsumerGroupMetadata(consumerGroupId)</code> to work with older brokers. See <a href="https://cwiki.apache.org/confluence/x/zJONCg">KIP-732</a> for more details.
</li>
+ <li>
+ The Connect <code>internal.key.converter</code> and <code>internal.value.converter</code> properties have been completely <a href="https://cwiki.apache.org/confluence/x/2YDOCg">removed</a>.
+ Workers are now hardcoded to use the JSON converter with <code>schemas.enable</code> set to <code>false</code>. If your cluster has been using
+ a different internal key or value converter, you can follow the migration steps outlined in <a href="https://cwiki.apache.org/confluence/x/2YDOCg">KIP-738</a>
+ to safely upgrade your Connect cluster to 3.0.
Review comment:
Might be good to mention that these have been deprecated since AK 2.0. This is similar to other items earlier in this list that mention the earlier version when defaults changed. Maybe something like:
```suggestion
The Connect <code>internal.key.converter</code> and <code>internal.value.converter</code> properties have been completely <a href="https://cwiki.apache.org/confluence/x/2YDOCg">removed</a>.
The use of these Connect worker properties has been deprecated since version 2.0.0.
Workers are now hardcoded to use the JSON converter with <code>schemas.enable</code> set to <code>false</code>. If your cluster has been using
a different internal key or value converter, you can follow the migration steps outlined in <a href="https://cwiki.apache.org/confluence/x/2YDOCg">KIP-738</a>
to safely upgrade your Connect cluster to 3.0.
```
##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java
##########
@@ -361,55 +324,23 @@ protected static ConfigDef baseConfigDef() {
.withClientSslSupport();
}
- private void logInternalConverterDeprecationWarnings(Map<String, String> props) {
- String[] deprecatedConfigs = new String[] {
- INTERNAL_KEY_CONVERTER_CLASS_CONFIG,
- INTERNAL_VALUE_CONVERTER_CLASS_CONFIG
- };
- for (String config : deprecatedConfigs) {
- if (props.containsKey(config)) {
- Class<?> internalConverterClass = getClass(config);
- logDeprecatedProperty(config, internalConverterClass.getCanonicalName(), INTERNAL_CONVERTER_DEFAULT.getCanonicalName(), null);
- if (internalConverterClass.equals(INTERNAL_CONVERTER_DEFAULT)) {
- // log the properties for this converter ...
- for (Map.Entry<String, Object> propEntry : originalsWithPrefix(config + ".").entrySet()) {
- String prop = propEntry.getKey();
- String propValue = propEntry.getValue().toString();
- String defaultValue = JsonConverterConfig.SCHEMAS_ENABLE_CONFIG.equals(prop) ? "false" : null;
- logDeprecatedProperty(config + "." + prop, propValue, defaultValue, config);
- }
- }
+ private void logInternalConverterRemovalWarnings(Map<String, String> props) {
+ List<String> removedProperties = new ArrayList<>();
+ for (String property : Arrays.asList("internal.key.converter", "internal.value.converter")) {
+ if (props.containsKey(property)) {
+ removedProperties.add(property);
}
+ removedProperties.addAll(originalsWithPrefix(property + ".").keySet());
}
- }
-
- private void logDeprecatedProperty(String propName, String propValue, String defaultValue, String prefix) {
- String prefixNotice = prefix != null
- ? " (along with all configuration for '" + prefix + "')"
- : "";
- if (defaultValue != null && defaultValue.equalsIgnoreCase(propValue)) {
- log.info(
- "Worker configuration property '{}'{} is deprecated and may be removed in an upcoming release. "
- + "The specified value '{}' matches the default, so this property can be safely removed from the worker configuration.",
- propName,
- prefixNotice,
- propValue
- );
- } else if (defaultValue != null) {
+ if (!removedProperties.isEmpty()) {
log.warn(
- "Worker configuration property '{}'{} is deprecated and may be removed in an upcoming release. "
- + "The specified value '{}' does NOT match the default and recommended value '{}'.",
- propName,
- prefixNotice,
- propValue,
- defaultValue
- );
- } else {
- log.warn(
- "Worker configuration property '{}'{} is deprecated and may be removed in an upcoming release.",
- propName,
- prefixNotice
- );
+ "The worker has been configured with one or more internal converter properties ({}). "
+ + "Support for these properties was dropped in version 3.0, and specifying them will "
Review comment:
WDYT about mentioning deprecation here, too, just for completeness?
Also, a tiny nit: maybe say "removed" rather than "dropped", since it's a bit more precise and it aligns with the upgrade docs (including mentions of other removals).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org