> On Nov. 30, 2015, 11:24 a.m., Amareshwari Sriramadasu wrote: > > lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java, > > line 24 > > <https://reviews.apache.org/r/40739/diff/1/?file=1147414#file1147414line24> > > > > Having cube.parse dependency from cube.metadata does not seem correct. > > > > If the required methods can be moved to a utility class in > > cube.metadata and used by both would be good. > > Rajat Khandelwal wrote: > It's only test code. Besides, we already have such a dependency. > https://github.com/apache/lens/blob/master/lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java#L24, > > https://github.com/apache/lens/blob/master/lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java#L38 > > Thirdly, CubeTestSetup is the origin of most cube test cases. More than a > few classes will depend on it to provide constants, static functions etc. > > Are you saying the original layout needs to be changed? If yes, can you > elaborate further? > > Amareshwari Sriramadasu wrote: > I see CubeTestSetup is mainly used for cube.parse tests and was lying in > cube.parse. package. The main intention i have metadata package should never > depend on parse/rewriting package. And i see there is a depedency of test > with source class as well. Shall we move Timerange class to metadata, if > range is used for timelines aswell? Also, move other required methods from > CubeTestSetup into a utility class in cube.metadata package. > > Actually, It would be better achived with module separation than through > review.
Replied separately. - Rajat ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40739/#review108288 ----------------------------------------------------------- On Nov. 30, 2015, 3:35 p.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40739/ > ----------------------------------------------------------- > > (Updated Nov. 30, 2015, 3:35 p.m.) > > > Review request for lens. > > > Bugs: LENS-885 > https://issues.apache.org/jira/browse/LENS-885 > > > Repository: lens > > > Description > ------- > > * Some functions are wrongly placed. e.g. DateUtil.getCeilDate(date, > updatePeriod) can be better expressed as updatePeriod.getCeilDate(date). > * Date parsing from query can be cached > * There is a bloat of Date variables in CubeTestSetup, organize them and only > keep named variables for the most used ones > * Unify time range clause creation across test cases. Right now, tests create > time ranges as and when needed. Remove repetition and redundancies. > * Correct names to variables > > > Diffs > ----- > > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeColumn.java > a2a00d2c60077a41eb9576901f1702fd94473cc7 > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java > d6bfb795373674f77705a2df14a9f24695a76a1a > lens-cube/src/main/java/org/apache/lens/cube/metadata/DateUtil.java > PRE-CREATION > > lens-cube/src/main/java/org/apache/lens/cube/metadata/TimePartitionRange.java > 01069a587c77230bddbae543eba799c2ce495a19 > lens-cube/src/main/java/org/apache/lens/cube/metadata/TimeRange.java > PRE-CREATION > lens-cube/src/main/java/org/apache/lens/cube/metadata/UpdatePeriod.java > 4c76a69d0e2986d450d81b8cbed877995696ba0d > > lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java > 9d5e264b725ba57ced7618dc2be9de9bcc7983f8 > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java > 7f8146158c1e2d883ee9327c4bf847883d0e1004 > > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java > 9c8b5b9fece146bcf0fa7d5048b93b236cd3d557 > lens-cube/src/main/java/org/apache/lens/cube/parse/DateUtil.java > 5e17eac62a6b12ef04d1d4e3254efbda54402eb6 > lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java > 200a48cd67e6a5a381ef3723a3adb686d8347efb > > lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactHQLContext.java > 60b2dde8a7e088d8712879c68cd98193bb72de03 > > lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java > cc8e68c5ad60edeb9638216a6cc9240750e3091e > lens-cube/src/main/java/org/apache/lens/cube/parse/TimeRange.java > 7be7ace884bb843604934f650c656e326767b7e3 > lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java > f772279712c3d1fc5da2325ec2e6a3c1402b550a > > lens-cube/src/test/java/org/apache/lens/cube/metadata/CubeFactTableTest.java > 2abc6d0b3d9d025543f09ca129bf9d351a188f25 > lens-cube/src/test/java/org/apache/lens/cube/metadata/DateFactory.java > PRE-CREATION > > lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java > e415a5adeb5a8af16bad56c96b9f9a32cb7afcbb > lens-cube/src/test/java/org/apache/lens/cube/metadata/TestDateUtil.java > PRE-CREATION > lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java > 1357035c8dfc6a5115c02d8fdc7cd8bb341eb6dd > > lens-cube/src/test/java/org/apache/lens/cube/parse/FieldsCannotBeQueriedTogetherTest.java > 0fea9f1bb4a266b680a252786ebd9e931d1b0c09 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java > 753ca331d28ad594555663d7f03374b75e5760f1 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java > ee84a4cdd5f0a54d90603c1ef96282e8068f4d32 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBetweenTimeRangeWriter.java > 9a2493c306938a0823d4b7d2bd264f915143ea64 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java > fea70b72cbde190d69d46f68d163fd9e541c53f8 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestDateUtil.java > ab88fbe067fadc839676f0013dc4885e3fbc5546 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java > d16ea4c56a792f1ac100e398bfdc6c8d1eab4490 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionContext.java > 0d1f9fe878aeec1161329d1ad4e8686fc6d65cd9 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java > 1e21fb0664d7d0153789cd47d14b4989b39bc5bb > lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java > ea561b6bd7bba05ebdd33bbe5f9785597459d732 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestORTimeRangeWriter.java > 4a23818313482d420721d62f908af4cc52f8311d > lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java > 255aade5e8b9c4d18ba20e4d263dcce0d497ce76 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestRewriterPlan.java > 4d3a3dc67ef0f18ef2354ac552fbc4e017022f17 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestStorageUtil.java > 73c3338f2b44f4d9e878a4de3d424f7184b9f9f0 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeExtractor.java > a4317173f9544ebcab05e290c3129479afd3bd6d > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java > 1fc8bc8ab7f617666b0d105d8c993dd354060c5e > lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeWriter.java > 0248409123f0588cbd5b45d0894bc9bf3503c888 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeWriterWithQuery.java > 7bd7b6b962fd9be83309ea3fc870dee9aefc2895 > > lens-server/src/main/java/org/apache/lens/server/query/QueryResultPurger.java > 54c657400d11c5eca8ff31ef954edc3c7ee53d1b > > Diff: https://reviews.apache.org/r/40739/diff/ > > > Testing > ------- > > > Thanks, > > Rajat Khandelwal > >
