feat: add data config checker for property value types #2328
+113
−40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrobinAV
reviewed
Dec 13, 2024
| if property_types := data_node_config._PROPERTIES_TYPES.get(data_node_config.storage_type): | ||
| for prop_key, prop_type in property_types.items(): | ||
| prop_value = data_node_config.properties.get(prop_key) if data_node_config.properties else None | ||
| if prop_value and not isinstance(prop_value, prop_type): |
There was a problem hiding this comment.
prop_type is the value in the _PROPERTIES_TYPES dictionary. Today, it must be a Python type.
Can we imagine prop_type being either a type or a list of types? Then, we should check that the type of prop_value is among the types in the prop_type list of types.
Does it make sense, or is it just over-engineering?
jrobinAV
added
Core
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this? (check all applicable)
Description
Related Tickets & Documents
How to reproduce the issue
Please replace this line with instructions on how to reproduce the issue or test the feature.
Other branches or releases that this needs to be backported
Describe which projects this change will impact and that needs to be backported.
Checklist
We encourage you to keep the code coverage percentage at 80% and above.