Skip to content

Stricter configuration validation - #6577

Open
ArbaazKhan1 wants to merge 1 commit into
apache:mainfrom
ArbaazKhan1:accumulo-6216
Open

ArbaazKhan1 wants to merge 1 commit into
apache:mainfrom
ArbaazKhan1:accumulo-6216

Conversation

@ArbaazKhan1

Copy link
Copy Markdown
Contributor

Closes issue #6216

  • Add new Property Type: CLOSED_PREFIX to represent non-extensible namespace prefixes
  • The existing PropertyType.PREFIX remains open for user-extensible properties (e.g. table.custom, table.iterator ...)
  • Reclassified tserver, manager, gc, monitor, rpc, sserver, compactor, compaction-coordinator, and instance to CLOSED_PREFIX
  • updated unit test to test for new closed prefix type
  • Updated ConfigCheckUtil, AccumuloConfiguration, ConfigurationImpl, ConfigurationDocGen, and SystemPropUtil to handle CLOSED_PREFIX wherever PropertyType.PREFIX was previously checked
  • Fix DefaultConfiguration static initializer to exclude CLOSED_PREFIX properties, which also have null default values
  • Unrecognized keys under closed prefix now log a warning instead of silently being accepted

@ArbaazKhan1

Copy link
Copy Markdown
Contributor Author

I know PR #6487 is also working on updating Properties. It looks like the work done there v here is mostly orthogonal, but depending on which get merged in first there will likely be some merge conflict, since they both touch the same block in Property.java. Worth double-checking once #6487 settles: if it ends up marking a property "required" just because its default value is null, that would wrongly flag every prefix property as required too, since both PREFIX and CLOSED_PREFIX types always have a null default by convention.

This branch has not been deployed

No deployments
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.

Make configuration validation more strict

2 participants