feat: adding validation for common recommended fields in sample #866
Conversation
harshilgajera-crest
changed the title
| if common_fields: | ||
| LOGGER.warning( | ||
| f"Common fields found in both cim_fields and missing_recommended_fields for {event.sample_name} sample = {common_fields}" | ||
| ) |
There was a problem hiding this comment.
Shouldn't we log an error and stop the execution if we something like this?
This would be technically a breaking change but I think we are fine with it - this would need to be documented properly in the PSA documentation.
There was a problem hiding this comment.
So if it is preset in both, then the tests would work as expected as it is appending both missing_recommended_fields and cim_fields at the end, so nothing would change from execution perspective.
There was a problem hiding this comment.
So actually functionally it doesn't change anything, beside suggestion that there are fields defined as both missing_recommended_fields and cim_fields right?
If so, then if we aim for "clean" samples, then I'd break tests execution, as I think nobody will really pay an attention to warning somewhere in the middle of PSA tests execution.
There was a problem hiding this comment.
So actually functionally it doesn't change anything, beside suggestion that there are fields defined as both missing_recommended_fields and cim_fields right?
Yes
If so, then if we aim for "clean" samples, then I'd break tests execution, as I think nobody will really pay an attention to warning somewhere in the middle of PSA tests execution.
Breaking test execution for something like this feels a bit unnecessary, but I do agree with part that warning would be hard to find between so many logs of PSA.
|
Closing this as this is implemented as a part of xml migration repo |
Added validation for same field present in cim_fields and missing_recommended_fields for sample and raise warning.