bob-barrett commented on PR #14721: URL: https://github.com/apache/kafka/pull/14721#issuecomment-1802992442
> Looks reasonable. Just want to confirm that all of these are thrown and not returned by any response apis? Good point, the `DynamicBrokerConfig` change can wind up bubbling up to https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/ConfigAdminManager.scala#L199 and getting returned as `INVALID_CONFIG` instead of `INVALID_REQUEST`, which the comment specifically calls out as something we shouldn't do. `SslFactory` can throw either on broker startup or in response to a dynamic config update, in which case it will actually wind up getting caught and re-thrown by `DynamicBrokerConfig`. None of the other changes can wind up in an API response. So for compatibility, we should drop the `DymamicBrokerConfig` change, but the others should be safe to keep. If we want to be extra safe about it, the only one I actually care about is `SslFactory`, I'm happy to drop the other changes. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
