platinumhamburg commented on code in PR #2279:
URL: https://github.com/apache/fluss/pull/2279#discussion_r2666855527
##########
fluss-server/src/main/java/org/apache/fluss/server/DynamicConfigManager.java:
##########
@@ -133,7 +133,7 @@ private void prepareIncrementalConfigs(
if (!dynamicServerConfig.isAllowedConfig(configPropName)) {
throw new ConfigException(
String.format(
- "The config key %s is not allowed to
be changed dynamically.",
+ "The config key %s is not allowed to
be changed dynamically or invalid (non-existent).",
configPropName));
Review Comment:
@sd4324530 Sorry, you misunderstood my point. What I meant was that, from a
testing perspective, the exposed error message doesn't accurately reflect the
underlying issue (and indeed, this wasn't introduced by your PR). I don't
recommend simply modifying the error message thrown in
DynamicConfigManager.java to meet the requirement, as this wouldn't actually
address the root problem—that the validation error messages aren't properly
categorized. If you don't intend to fix this issue now, I suggest converting
the comment into a TODO note explaining the situation and reverting the changes
made in DynamicConfigManager.java.
Additionally, I think the PR is overall quite mature and, after addressing
these minor adjustments, it should be ready to move into the final review stage.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
