Skip to content

Improve Property Types #6487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Amemeda wants to merge 7 commits into
base: main
Choose a base branch
from
+80 −22
Draft

Improve Property Types #6487

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a702db0
Move required properties out of ServerConfigCheckRunner
Amemeda Jul 21, 2026
a44536b
Fix incorrect return type in comment
Amemeda Jul 21, 2026
f9d82be
Add validation for PropertyType.URI and PropertyType.PATH
Amemeda Jul 22, 2026
a77bedb
Fix failing unit test for PropertyType.PATH
Amemeda Jul 22, 2026
b993313
Fix checkstyle violdations
Amemeda Jul 23, 2026
c499e66
Remove outdated TODO from PropertyType.java, apache#2699
Amemeda Jul 23, 2026
335922d
Simplify matches regex for PropertyType.PATH validation
Amemeda Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions core/src/main/java/org/apache/accumulo/core/conf/Property.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1564,6 +1564,23 @@ public Property replacedBy() {
return replacedBy;
}

/**
* Gets the defined required properties
*
* @return Set<Property>
*/
public static Set<Property> getRequiredProperties() {
return Set.of(Property.INSTANCE_ZK_HOST, Property.INSTANCE_ZK_TIMEOUT,
Property.INSTANCE_SECRET, Property.INSTANCE_VOLUMES, Property.GENERAL_THREADPOOL_SIZE,
Property.GENERAL_DELEGATION_TOKEN_LIFETIME,
Property.GENERAL_DELEGATION_TOKEN_UPDATE_INTERVAL, Property.GENERAL_IDLE_PROCESS_INTERVAL,
Property.GENERAL_LOW_MEM_DETECTOR_INTERVAL, Property.GENERAL_LOW_MEM_DETECTOR_THRESHOLD,
Property.GENERAL_SERVER_LOCK_VERIFICATION_INTERVAL, Property.MANAGER_CLIENTPORT,
Property.TSERV_CLIENTPORT, Property.GC_CYCLE_START, Property.GC_CYCLE_DELAY,
Property.GC_PORT, Property.MONITOR_PORT, Property.TABLE_MAJC_RATIO,
Property.TABLE_SPLIT_THRESHOLD);
}
Comment on lines +1572 to +1582

@Amemeda Amemeda Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any more required properties missing? Or how can I tell if the property is required?


private void precomputeAnnotations() {
isSensitive =
hasAnnotation(Sensitive.class) || hasPrefixWithAnnotation(getKey(), Sensitive.class);
Expand Down
14 changes: 1 addition & 13 deletions ...e/src/main/java/org/apache/accumulo/server/util/checkCommand/ServerConfigCheckRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,7 @@ public boolean runCheck(ServerContext context, ServerOpts opts, boolean fixFiles
}

log.trace("Checking that all required config properties are present");
// there are many properties that should be set (default value or user set), identifying them
// all and checking them here is unrealistic. Some property that is not set but is expected
// will likely result in some sort of failure eventually anyway. We will just check a few
// obvious required properties here.
Set<Property> requiredProps = Set.of(Property.INSTANCE_ZK_HOST, Property.INSTANCE_ZK_TIMEOUT,
Property.INSTANCE_SECRET, Property.INSTANCE_VOLUMES, Property.GENERAL_THREADPOOL_SIZE,
Property.GENERAL_DELEGATION_TOKEN_LIFETIME,
Property.GENERAL_DELEGATION_TOKEN_UPDATE_INTERVAL, Property.GENERAL_IDLE_PROCESS_INTERVAL,
Property.GENERAL_LOW_MEM_DETECTOR_INTERVAL, Property.GENERAL_LOW_MEM_DETECTOR_THRESHOLD,
Property.GENERAL_SERVER_LOCK_VERIFICATION_INTERVAL, Property.MANAGER_CLIENTPORT,
Property.TSERV_CLIENTPORT, Property.GC_CYCLE_START, Property.GC_CYCLE_DELAY,
Property.GC_PORT, Property.MONITOR_PORT, Property.TABLE_MAJC_RATIO,
Property.TABLE_SPLIT_THRESHOLD);
Set<Property> requiredProps = Property.getRequiredProperties();
for (var reqProp : requiredProps) {
var confPropVal = config.get(reqProp);
// already checked that all set properties are valid, just check that it is set then we know
Expand Down
Loading