Skip to content
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.

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

Allow setting validation against arbitrary types #47264

Merged
merged 12 commits into from
Oct 2, 2019

Conversation

jasontedor
Copy link
Member

@jasontedor jasontedor commented Sep 28, 2019

Today when settings validate, they can only validate against settings that are of the same type. While this strong-type is convenient from a development perspective, it is too limiting in that some settings need to validate against settings of a different type. For example, the list setting xpack.monitoring.exporters.<namespace>.host wants to validate that it is non-empty if and only if the string setting xpack.monitoring.exporters.<namespace>.type is "http". Today this is impossible since the settings validation framework only allows that setting to validate against other list settings. This commit increases the flexibility here to validate against settings of arbitrary type, at the expense of losing strong-typing during development.

Relates #25560

Today when settings validate, they can only validate against settings
that are of the same type. While this strong-type is convenient from a
development perspective, it is too limiting in that some settings need
to validate against settings of a different type. For example, the list
setting xpack.monitoring.exporters.<namespace>.host wants to validate
that it is non-empty if and only if the string setting
xpack.monitoring.exporters.<namespace>.type is "http". Today this is
impossible since the settings validation framework only allows that
setting to validate against other list settings. This commit increases
the flexibility here to validate against settings of arbitrary type, at
the expense of losing strong-typing during development.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jasontedor
Copy link
Member Author

@elasticmachine run elasticsearch-ci/2

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM. Long term I think we should continue experimenting with how cross validation works, so that we have better type safety (ie not requiring casting to extract values), but this isn't really any worse than we already had.

@jasontedor jasontedor merged commit f63ee2f into elastic:master Oct 2, 2019
jasontedor added a commit that referenced this pull request Oct 2, 2019
Today when settings validate, they can only validate against settings
that are of the same type. While this strong-type is convenient from a
development perspective, it is too limiting in that some settings need
to validate against settings of a different type. For example, the list
setting xpack.monitoring.exporters.<namespace>.host wants to validate
that it is non-empty if and only if the string setting
xpack.monitoring.exporters.<namespace>.type is "http". Today this is
impossible since the settings validation framework only allows that
setting to validate against other list settings. This commit increases
the flexibility here to validate against settings of arbitrary type, at
the expense of losing strong-typing during development.
@jasontedor jasontedor deleted the arbitrary-settings-validation branch October 2, 2019 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants