-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review162787
-----------------------------------------------------------




lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java (line 
37)
<https://reviews.apache.org/r/55712/#comment234139>

    Please update the comment to state the value is table_name_prefix



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java (line 
39)
<https://reviews.apache.org/r/55712/#comment234137>

    Lets change the map to Map<String, Map<UpdatePeriod, String>>



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java (line 
111)
<https://reviews.apache.org/r/55712/#comment234138>

    Should this map be populated for Facts which are created before this 
feature as well?



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 
(line 189)
<https://reviews.apache.org/r/55712/#comment234141>

    Is the method giving StorageTableName or storageTablePrefix ? Please name 
method or variable accordingly.



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 
(line 192)
<https://reviews.apache.org/r/55712/#comment234142>

    Why is storageTablePrefix required for look up on partitionTimelineCache ?
    
    what is passed for key?



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 
(lines 225 - 229)
<https://reviews.apache.org/r/55712/#comment234143>

    Did not understand why these code changes are required.



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 
(line 345)
<https://reviews.apache.org/r/55712/#comment234144>

    We need understand why storagePrefix is required for PartitionTimeline.
    
    I feel it should not be required.



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 
(line 754)
<https://reviews.apache.org/r/55712/#comment234145>

    Can you update the comment to what is it now?



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 
(line 973)
<https://reviews.apache.org/r/55712/#comment234146>

    Why is getPrefix method taking storageTableName as parameter?



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 
(line 977)
<https://reviews.apache.org/r/55712/#comment234147>

    Same as above, why is getPrefix taking storageTableName as prefix?



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 
(line 2275)
<https://reviews.apache.org/r/55712/#comment234149>

    How will this work if is same table is mapped for all update periods?



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 
(line 2278)
<https://reviews.apache.org/r/55712/#comment234148>

    I see drop is already happening before.



lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 
(line 2490)
<https://reviews.apache.org/r/55712/#comment234150>

    Please name the variable and method appropriately. And should this method 
be moved to CubeFactTable?



lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java (line 133)
<https://reviews.apache.org/r/55712/#comment234153>

    Should we name the param as storageTableNamePrefix? Also, please update 
javadoc accordingly.



lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java (line 266)
<https://reviews.apache.org/r/55712/#comment234154>

    Rename param to storageTableNamePrefix?



lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java (line 387)
<https://reviews.apache.org/r/55712/#comment234155>

    Rename param to storageTableNamePrefix?



lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java (line 
860)
<https://reviews.apache.org/r/55712/#comment234156>

    Can we rename the map appropriately?


- Amareshwari Sriramadasu


On Jan. 23, 2017, 2:51 p.m., Lavkesh Lahngir wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2017, 2:51 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1386
>     https://issues.apache.org/jira/browse/LENS-1386
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> A new data structure XUpdatePeriodTableDescriptor is introduced which 
> contains an update period and table descriptor. Now the XUpdatePeriods will 
> contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/resources/cube-0.1.xsd f438f48 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java 
> adb6c92 
>   
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java
>  6c9cde2 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 
> 53cf8af 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705 
>   
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java
>  e21dc2a 
>   
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java
>  8b10d1d 
>   lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 
> 51fcb43 
> 
> Diff: https://reviews.apache.org/r/55712/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>

Reply via email to