You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "chia7712 (via GitHub)" <gi...@apache.org> on 2025/09/11 22:30:43 UTC
Re: [PR] MINOR: Refactor on FeaturesPublisher and ScramPublisher [kafka]
chia7712 commented on code in PR #20522:
URL: https://github.com/apache/kafka/pull/20522#discussion_r2342470917
##########
metadata/src/main/java/org/apache/kafka/metadata/publisher/FeaturesPublisher.java:
##########
@@ -54,15 +58,20 @@ public void onMetadataUpdate(
MetadataImage newImage,
LoaderManifest manifest
) {
- if (delta.featuresDelta() != null) {
- FinalizedFeatures newFinalizedFeatures = new FinalizedFeatures(newImage.features().metadataVersionOrThrow(),
- newImage.features().finalizedVersions(),
- newImage.provenance().lastContainedOffset()
- );
- if (!newFinalizedFeatures.equals(finalizedFeatures)) {
- log.info("Loaded new metadata {}.", newFinalizedFeatures);
- finalizedFeatures = newFinalizedFeatures;
+ try {
+ if (delta.featuresDelta() != null) {
+ FinalizedFeatures newFinalizedFeatures = new FinalizedFeatures(newImage.features().metadataVersionOrThrow(),
+ newImage.features().finalizedVersions(),
+ newImage.provenance().lastContainedOffset()
+ );
+ if (!newFinalizedFeatures.equals(finalizedFeatures)) {
+ log.info("Loaded new metadata {}.", newFinalizedFeatures);
+ finalizedFeatures = newFinalizedFeatures;
+ }
}
+ } catch (Throwable t) {
+ String deltaName = "MetadataDelta up to " + newImage.highestOffsetAndEpoch().offset();
+ faultHandler.handleFault("Uncaught exception while publishing features changes from " + deltaName, t);
Review Comment:
we could concat the strings directly, right? for example:
```java
faultHandler.handleFault("Uncaught exception while publishing SCRAM changes from MetadataDelta up to "
+ newImage.highestOffsetAndEpoch().offset(), t);
```
##########
metadata/src/main/java/org/apache/kafka/metadata/publisher/ScramPublisher.java:
##########
@@ -63,6 +58,7 @@ public void onMetadataUpdate(MetadataDelta delta, MetadataImage newImage) {
});
}
} catch (Throwable t) {
+ String deltaName = "MetadataDelta up to " + newImage.highestOffsetAndEpoch().offset();
Review Comment:
ditto
##########
metadata/src/main/java/org/apache/kafka/metadata/publisher/FeaturesPublisher.java:
##########
@@ -54,15 +58,20 @@ public void onMetadataUpdate(
MetadataImage newImage,
LoaderManifest manifest
) {
- if (delta.featuresDelta() != null) {
- FinalizedFeatures newFinalizedFeatures = new FinalizedFeatures(newImage.features().metadataVersionOrThrow(),
- newImage.features().finalizedVersions(),
- newImage.provenance().lastContainedOffset()
- );
- if (!newFinalizedFeatures.equals(finalizedFeatures)) {
- log.info("Loaded new metadata {}.", newFinalizedFeatures);
- finalizedFeatures = newFinalizedFeatures;
+ try {
+ if (delta.featuresDelta() != null) {
+ FinalizedFeatures newFinalizedFeatures = new FinalizedFeatures(newImage.features().metadataVersionOrThrow(),
+ newImage.features().finalizedVersions(),
Review Comment:
could you please remove the extra indent?
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org