Merged in conditional_requiredness (pull request #42)
bf9d84b·Author: Arthur Eigenbrot·Closed by: Arthur Eigenbrot·2024-01-12
Description
We have recently been bit by an issue where certain L1 header (i.e., SPEC-214) keys are only required sometimes; this lead to a situation where L1 files were passing validation while missing crucial keys. The root problem is that, until now, the required spec field is fixed for all time, so if a key is only required sometimes then in the spec it has required: false.
This PR introduces a framework for specifying conditional requiredness that will override the baked in required value. The pattern is very similar to the pattern for expansion keys because in both cases we are modifying the spec based on values in an actual header.
Key changes:
Add ConditionalRequirement base class and update_schema_requiredness for defining and implementing conditional requirednessess.update_schema_requiredness
The frameworks for both expansion and conditional requiredness have been moved to the new spec_processors module.
load_expanded_spec214, which had been the top-level “gimme the dang spec” function used by most libraries has been replaced by the more generic load_processed_spec214 function, which does both expansion and requiredness updating. This is also a great place to put any future spec-processing steps. This is a massively breaking change for all downstream services, though. Watch out!
In addition to the framework itself, this PR demonstrates its usage by defining a conditional requirement that forces all keys in an instrument table to be required if given a header taken with that instrument. Yes, it’s crazy that this wasn’t true before, but the fact that it was really highlights the limitations of a fixed-requiredness paradigm.
The diff is kinda jacked in BB so I’ll try to add some clarifying comments below.
We have recently been bit by an issue where certain L1 header (i.e., SPEC-214) keys are only required sometimes; this lead to a situation where L1 files were passing validation while missing crucial keys. The root problem is that, until now, the
requiredspec field is fixed for all time, so if a key is only required sometimes then in the spec it hasrequired: false.This PR introduces a framework for specifying conditional requiredness that will override the baked in
requiredvalue. The pattern is very similar to the pattern for expansion keys because in both cases we are modifying the spec based on values in an actual header.Key changes:
Add
ConditionalRequirementbase class andupdate_schema_requirednessfor defining and implementing conditional requirednessess.update_schema_requirednessThe frameworks for both expansion and conditional requiredness have been moved to the new
spec_processorsmodule.load_expanded_spec214, which had been the top-level “gimme the dang spec” function used by most libraries has been replaced by the more genericload_processed_spec214function, which does both expansion and requiredness updating. This is also a great place to put any future spec-processing steps. This is a massively breaking change for all downstream services, though. Watch out!In addition to the framework itself, this PR demonstrates its usage by defining a conditional requirement that forces all keys in an instrument table to be required if given a header taken with that instrument. Yes, it’s crazy that this wasn’t true before, but the fact that it was really highlights the limitations of a fixed-requiredness paradigm.
The diff is kinda jacked in BB so I’ll try to add some clarifying comments below.