You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "yyqdbngt (via GitHub)" <gi...@apache.org> on 2026/08/31 19:01:15 UTC
[PR] fix(settings): preserve unmanaged settings fields on partial saves [rocketmq-dashboard]
yyqdbngt opened a new pull request, #2863:
URL: https://github.com/apache/rocketmq-dashboard/pull/2863
## Summary
- In `SettingsService.saveGeneralSettings`, inherit the stored value for fields that a
partial writer does not manage: `dingtalkWebhook`, `smsWebhook`, `emailRecipients`,
`llmEngine`, `maxTokens`, `temperature`.
- An absent (`null`) value now inherits the stored one; an explicit empty string still
clears a text field, so the existing clear-by-empty-string behaviour is unchanged.
- Added regression tests for both partial-writer directions and the explicit-clear case.
## Why
The persisted settings blob has two writers that each manage only part of the fields:
- the general-settings save (`/api/settings/general/save`, `GeneralSettingsUpdateDTO`)
has no `maxTokens`/`temperature` properties, so every general save nulled the LLM
tuning and the next LLM config load silently fell back to the defaults;
- the LLM config save (`LlmConfigService.saveConfig`) builds the VO without
`dingtalkWebhook`/`smsWebhook`/`emailRecipients`/`llmEngine`, so saving the LLM
config wiped the notification channels and engine override.
Both were data-loss-on-save bugs in the opposite direction of each other.
## Testing
- `cd server && mvn -Dtest=SettingsServiceTest test` — Tests run: 46, Failures: 0, Errors: 0
- `cd server && mvn -Dtest=LlmConfigServiceTest test` — Tests run: 28, Failures: 0, Errors: 0
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(settings): preserve unmanaged settings fields on partial saves [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2863:
URL: https://github.com/apache/rocketmq-dashboard/pull/2863#issuecomment-5611297396
This pull request has been automatically closed because there has been no activity for 7 days. If you would like to continue working on this, please reopen the PR with updated 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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(settings): preserve unmanaged settings fields on partial saves [rocketmq-dashboard]
Posted by "RockteMQ-AI (via GitHub)" <gi...@apache.org>.
RockteMQ-AI commented on code in PR #2863:
URL: https://github.com/apache/rocketmq-dashboard/pull/2863#discussion_r3897741852
##########
server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java:
##########
@@ -147,6 +147,28 @@ public synchronized void saveGeneralSettings(GeneralSettingsVO settings) {
if (!StringUtils.hasText(settings.getAwsRegion())) {
settings.setAwsRegion(currentSettings.getAwsRegion());
}
+ // The settings blob has two partial writers: the general-settings save does not
+ // manage the LLM tuning values, and the LLM-config save does not manage the
+ // notification fields. An absent (null) value therefore inherits the stored one,
+ // while an explicit empty string still clears a text field.
Review Comment:
Good documentation of the two-writer invariant. One consideration: if more fields are added later, the null-check boilerplate will grow linearly. A small helper method could keep this maintainable. Not blocking — just a thought for future iterations.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(settings): preserve unmanaged settings fields on partial saves [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #2863: fix(settings): preserve unmanaged settings fields on partial saves
URL: https://github.com/apache/rocketmq-dashboard/pull/2863
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org