----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65742/#review198099 -----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java Lines 221 (patched) <https://reviews.apache.org/r/65742/#comment278209> Do we handle 5 classification of the same type against an entity, but with non-overlapping validity periods. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java Lines 222 (patched) <https://reviews.apache.org/r/65742/#comment278208> My feeling is that there should be a new state for the classification that says it is yet to be activated. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java Lines 226 (patched) <https://reviews.apache.org/r/65742/#comment278207> We should have checking to make sure the lower bounds are sooner than the upper bounds. Do we allow the same value for bottom and top. Do we allow adding classification validity periods in the past? Or crossing the present? Maybe we round the time period to a minute to prevent timeing issues for very small time periods. I think we should police all of these bioundary conditions and have unique messages for each case. Holding and specifying the time in UTC would prevent any quirky issues like daylight savings, 29th of Feb is valid only for leap years. - David Radley On Feb. 22, 2018, 12:19 a.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65742/ > ----------------------------------------------------------- > > (Updated Feb. 22, 2018, 12:19 a.m.) > > > Review request for atlas. > > > Bugs: ATLAS-2457 > https://issues.apache.org/jira/browse/ATLAS-2457 > > > Repository: atlas > > > Description > ------- > > Updated AtlasClassification with addition of 'timeBoundary' attribute. > > > Diffs > ----- > > common/src/main/java/org/apache/atlas/repository/Constants.java ae528807 > intg/src/main/java/org/apache/atlas/model/TimeBoundary.java PRE-CREATION > intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java > a499f793 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java > 0224bf01 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java > d01fb9f0 > > webapp/src/test/java/org/apache/atlas/web/integration/EntityV2JerseyResourceIT.java > dabb2efa > > > Diff: https://reviews.apache.org/r/65742/diff/2/ > > > Testing > ------- > > - added IT to validate that the new attribute is correctly stored/retrieved > > > Thanks, > > Madhan Neethiraj > >
