Skip to content

Conversation

@alexandernorth
Copy link
Contributor

This is a quick implementation of a way to disable validators based on the data-server.yaml config file to unblock other users. A refactoring of the validation logic is planned.

Adding the following lines to the root level of the data-server.yaml file will disable all validators:

validation:
  disabled-validators:
    - mandatory
    - leafref
    - leafref-min-max-attributes
    - pattern
    - must-statements
    - length
    - range
#    - max-elements

@steiler
Copy link
Collaborator

steiler commented Mar 11, 2025

Thanks Alex.

  1. I'd rather go for a struct with explicit validator names.
validator:
   disable_validation:
      must_statements: true
      leafref: true

This would then result in all the fields being false by default and only explicitly true fields are true.
2. Also the global var AvailableValidators I would try to avoid. Globals are usually not to be favoured. Same for LoadedConfig. So add a field to datastore.New that injects the config validation struct and add a parameter to the validation func that takes the config struct as a parameter as well.
4. We should maybe in the config create a datastore top level, push the datastores under it and have the validation also rooted there.

@steiler
Copy link
Collaborator

steiler commented Mar 11, 2025

lgtm

Copy link
Contributor

@hansthienpondt hansthienpondt left a comment

Choose a reason for hiding this comment

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

LGTM

@alexandernorth alexandernorth force-pushed the feature/config-params-for-validation branch from 44164fd to ff6dfde Compare March 11, 2025 15:51
@hansthienpondt hansthienpondt merged commit b2d33fe into sdcio:main Mar 12, 2025
4 of 5 checks passed
@alexandernorth alexandernorth deleted the feature/config-params-for-validation branch March 12, 2025 09:53
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.

3 participants