> On May 18, 2015, 8:08 a.m., Rajat Khandelwal wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/common/Constant.java,
> >  line 21
> > <https://reviews.apache.org/r/34210/diff/2/?file=959324#file959324line21>
> >
> >     Rename this to something other than the all-generic `Constant`. It 
> > doesn't yet contain every possible `Constant` to be eligible for the name 
> > `Constant`.
> 
> Himanshu Gahlaut wrote:
>     Couldn't figure out anything common between REQUEST_ID and 
> LOG_SEGREGATION_ID to assign a common name to them. Please feel free to 
> suggest a name which can relate these two constants. Putting both of them in 
> separate enums will be enum bloat. Hence started a generic common Constant 
> enum in lens-server-api which doesn't exist right now.
> 
> Rajat Khandelwal wrote:
>     so REQUEST_ID is used in only one place, it can move there. 
> `LOG_SEGGREGATION_ID` is only used like the following:
>     
>         MDC.put(Constant.LOG_SEGREGATION_ID.getValue(), something)
>     
>     or
>         MDC.get(Constant.LOG_SEGREGATION_ID.getValue());
>     
>     
>     These methods can be abstracted out in a new or old class(say, 
> `LogHelper`) which can internally store the constant.

will move REQUEST_ID to LensRequestContextInitFilter. Helper classes tends to 
become code smell in longer run. Will encapsulate log segregation into a domain 
problem and provide domain objects for the same.


- Himanshu


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


On May 14, 2015, 2:06 p.m., Himanshu Gahlaut wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34210/
> -----------------------------------------------------------
> 
> (Updated May 14, 2015, 2:06 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-514
>     https://issues.apache.org/jira/browse/LENS-514
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> LENS-514: Adding unique identifier to log lines for log segregation
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/java/org/apache/lens/api/query/QueryHandle.java 
> 7b615e6216d7a31a743416dd839a63517be88e1f 
>   lens-api/src/main/java/org/apache/lens/api/query/QueryPrepareHandle.java 
> efa04f1c7e7fddbd9176922425c66057f4be3089 
>   lens-api/src/main/java/org/apache/lens/api/response/LensResponse.java 
> f6c3593a131ab6779186be014d79b9f4c4cb9534 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/common/Constant.java 
> PRE-CREATION 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/query/PreparedQueryContext.java
>  1ce89ac66fa817b3e47482a81b040259d2194477 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java
>  169ac8dec4b98a0cab1f652fe85afe9737f02e39 
>   lens-server/src/main/java/org/apache/lens/server/AuthenticationFilter.java 
> b64d8223727d755aad95dee288f9561b767a5448 
>   lens-server/src/main/java/org/apache/lens/server/InitialRequestFilter.java 
> PRE-CREATION 
>   lens-server/src/main/java/org/apache/lens/server/LensApplication.java 
> cb452e83e6edbe4f13a3f29b4ce0cbe45effcde3 
>   lens-server/src/main/java/org/apache/lens/server/LensServer.java 
> c6d7ea154f4c3f78eaa0675d99364a311351bf4c 
>   lens-server/src/main/java/org/apache/lens/server/query/QueryApp.java 
> 0d23726a895a11208249b7ce458cb34e2265feeb 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  3bf180c9ab6f2a9fec6259848ebf65ecd147cfcf 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryServiceResource.java
>  9b6d6bc223069b28367d789db847cbf05ed386c7 
>   
> lens-server/src/test/java/org/apache/lens/server/query/QueryAPIErrorResponseTest.java
>  5e135be0348068125b69016274e00b01140c4f21 
>   lens-server/src/test/resources/log4j.properties 
> 65d065fa699f9797dcb13580359c4d41cfc52e9d 
>   tools/conf-pseudo-distr/server/log4j.properties 
> afadc2f2856e7723e23265553440984bf2859869 
>   tools/conf/server/log4j.properties afadc2f2856e7723e23265553440984bf2859869 
> 
> Diff: https://reviews.apache.org/r/34210/diff/
> 
> 
> Testing
> -------
> 
> Testing done by executing query from lens-cli and verifying lens-server logs.
> 
> Running test suite. Will update test results.
> 
> 
> Thanks,
> 
> Himanshu Gahlaut
> 
>

Reply via email to