Skip to content

Improve Property Types#6487

Draft
Amemeda wants to merge 4 commits into
apache:mainfrom
Amemeda:property-type
Draft

Improve Property Types#6487
Amemeda wants to merge 4 commits into
apache:mainfrom
Amemeda:property-type

Conversation

@Amemeda

@Amemeda Amemeda commented Jul 21, 2026

Copy link
Copy Markdown
Contributor
  • Moved required properties out of ServerConfigCheckRunner.java and into Property.java as a static method that returns a Set of the required properties
  • Added validation for PropertyType.PATH and PropertyType.URI

This pr resolves #6112


TODO:

  • May need to add more properties to the required properties set
  • Replace 46 occurrences of PropertyType.STRING with appropriate new/existing PropertyTypes for proper validation
  • Analyze all properties, determine if they are required, update validation
    - Properties that are not required could accept empty string, null, or a valid value (where validity is well defined)
    - Properties that are required could only accept a valid value

Refer to discussion over property types on pr #5348

@Amemeda
Amemeda marked this pull request as ready for review July 21, 2026 18:03
Comment on lines +1572 to +1582
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);
}

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?

@Amemeda
Amemeda marked this pull request as draft July 21, 2026 18:04
@Amemeda

Amemeda commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

@kevinrr888 Checking if you are still available for discussion on this issue, Looking for your guidance/opinion over the expected changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Property types can be improved

1 participant