[scd] Validate Volume4D when parsing#1380
Conversation
RustedBones
force-pushed
the
volume-validation
branch
5 times, most recently
from
51e5d77 to
9a11fcb
Compare
RustedBones
force-pushed
the
volume-validation
branch
from
9a11fcb to
6ff791d
Compare
mickmis
left a comment
mickmis
left a comment
There was a problem hiding this comment.
Intent SGTM, I do have suggestions regarding implementation, please LMK what you think.
mickmis
left a comment
mickmis
left a comment
There was a problem hiding this comment.
LGTM modulo comment on SpatialVolume
| return nil, stacktrace.NewError("Missing time_end from extents") | ||
| } | ||
|
|
||
| if now.After(*valid.uExtent.EndTime) { |
There was a problem hiding this comment.
nit, not for this PR: pushing it further this could be a validator WithRequireEndTimeBefore(now)
There was a problem hiding this comment.
Just to be sure we're alligned on this one, the doc states:
Spacetime extents that bound this operational intent. Start and end times, as well as lower and upper altitudes, are required for each volume. The end time may not be in the past.
I think to move the validation on the unioned extent so we can accept composite extens with some expired volumes but not all. is that correct ?
There was a problem hiding this comment.
Missed that comment - my bad.
2 participants
Factorize validation for
Volume3DandVolume4Dduring parsing:startTime<endTimewhen definedaltLo<altHiwhen definedIntroduce parsing options to tune validation:
altLoandaltHiset whenRequireAltitudeBoundsenabledstartTimeansendTimeset whenRequireTimeBoundsenabledConfiguration options for SCD entities mutation:
RequireTimeBounds: true, RequireAltitudeBounds: trueRequireTimeBounds: true, RequireAltitudeBounds: falseRequireTimeBounds: false, RequireAltitudeBounds: falseFor SCD queries:
RequireTimeBounds: false, RequireAltitudeBounds: false