[WIP] Configuration settings validation improvements#10998
[WIP] Configuration settings validation improvements#10998sureshanaparti wants to merge 1 commit intoapache:4.19from
Conversation
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.19 #10998 +/- ##
============================================
+ Coverage 4.28% 15.17% +10.89%
- Complexity 0 11367 +11367
============================================
Files 372 5416 +5044
Lines 29745 476105 +446360
Branches 5229 58134 +52905
============================================
+ Hits 1275 72270 +70995
- Misses 28324 395744 +367420
- Partials 146 8091 +7945
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13689 |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances configuration settings validation and cleans up redundant configuration declarations, while refactoring related validation logic. Key changes include:
- Removal of duplicate configuration keys and unused variables.
- Improvements in type handling, especially for Boolean and Double values.
- Refactoring of configuration value parsing by introducing a helper method for consistent error messaging.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/src/main/java/com/cloud/storage/ImageStoreServiceImpl.java | Removed unused ConfigKey and variable declarations for image storage imbalance threshold. |
| server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java | Updated Boolean parsing to better handle null values. |
| server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java | Refactored configuration validation logic with a new helper method; added range checking for Double values. |
| server/src/main/java/com/cloud/configuration/Config.java | Updated configuration type for load balancer enabled flag from String to Boolean. |
| plugins/database/quota/.../QuotaResponseBuilderImpl.java | Adjusted config value handling to use Boolean directly. |
| framework/quota/.../QuotaConfig.java | Updated types for quota configuration keys from String to Boolean. |
| engine/orchestration/.../StorageOrchestrator.java | Removed redundant configuration key declaration. |
| engine/api/.../StorageOrchestrationService.java | Centralized declaration of image store imbalance threshold via the interface. |
| if (type.equals(Double.class)) { | ||
| try { | ||
| final Double val = Double.parseDouble(value); | ||
| if (weightBasedParametersForValidation.contains(name) && (val < 0f || val > 1f)) { |
There was a problem hiding this comment.
Consider using double literals (0.0 and 1.0) instead of float literals (0f and 1f) for clarity when comparing Double values.
| if (weightBasedParametersForValidation.contains(name) && (val < 0f || val > 1f)) { | |
| if (weightBasedParametersForValidation.contains(name) && (val < 0.0 || val > 1.0)) { |
b7a23e2 to
dfab15b
Compare
|
@sureshanaparti do you still want to continue with this? |
Description
This PR improves configuration settings validation and some code refactoring.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?