> On Feb. 22, 2018, 9:26 a.m., David Radley wrote: > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java > > Lines 221 (patched) > > <https://reviews.apache.org/r/65742/diff/2/?file=1963341#file1963341line223> > > > > Do we handle 5 classification of the same type against an entity, but > > with non-overlapping validity periods.
Having a classification associated with an entity multiple times can be messy to deal with - in Atlas and applications that deal with the classifications. Instead how about allowing a list of validityPeriods in a classification, instead of a single validityPeriod? > On Feb. 22, 2018, 9:26 a.m., David Radley wrote: > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java > > Lines 222 (patched) > > <https://reviews.apache.org/r/65742/diff/2/?file=1963341#file1963341line224> > > > > My feeling is that there should be a new state for the classification > > that says it is yet to be activated. Maintaining such a state field would require Atlas to run a background task to monitor all clasifications and update their state. This may not be of much value for Atlas. Hence, Atlas doesn't interpret the validityPeriods associated with classifications; applications that consume classifications, like Ranger, are better place for this. > On Feb. 22, 2018, 9:26 a.m., David Radley wrote: > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java > > Lines 226 (patched) > > <https://reviews.apache.org/r/65742/diff/2/?file=1963341#file1963341line228> > > > > 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. As I mentioned in the previous comment, Atlas doesn't interpret the validityPeriod - hence there are no validations in place. Howver, I agree that basic validations like format of startTime/endTime attributes, endTime later than startTime can be performed. I thought about using UTC time, but that will make it difficult to model "local-time" scenarios, for example: startTime="2018/03/01 09:00:00" endTime="2018/03/01 17:00:00" If these times need to be interpreted as local-time i.e whereever the classification is consumed (consider Ranger policy-engine running different timezones: GMT, Pacific, Eastern), use of UTC time won't work. - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65742/#review198099 ----------------------------------------------------------- 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 > >
