> On July 4, 2016, 10:13 a.m., Shwetha GS wrote:
> > addons/falcon-bridge-shim/src/main/java/org/apache/atlas/falcon/hook/AtlasService.java,
> >  line 31
> > <https://reviews.apache.org/r/49585/diff/2/?file=1436099#file1436099line31>
> >
> >     Does falcon-shim/AtlasService and falcon/AtasService need to have same 
> > name and package? If not, can you rename them as its confusing
> >     
> >     For example, falcon-shim/AtlasService can be renamed to 
> > AtlasProxyService
> 
> Madhan Neethiraj wrote:
>     With the use of classname/package for the shim and the implementation, no 
> changes are needed in the hook registration in components 
> (Falcon/Hive/Sqoop/Storm). Hence this choice.
> 
> Madhan Neethiraj wrote:
>     Typo: "With the use of classname/package" ==> "With the use of same 
> classname/package"

Lets rename falcon/AtasService then. That wouldn't require changes in hook 
configuration, right?


> On July 4, 2016, 10:13 a.m., Shwetha GS wrote:
> > addons/falcon-bridge-shim/src/main/java/org/apache/atlas/falcon/hook/AtlasService.java,
> >  line 46
> > <https://reviews.apache.org/r/49585/diff/2/?file=1436099#file1436099line46>
> >
> >     We should use aspectj and remove loggers and activate and deactive. If 
> > you can't address in this patch, can you file a bug?
> 
> Madhan Neethiraj wrote:
>     The comment does not seem related to this source line. Can you review?

Every method in falcon-shim/AtlasService has this logic - debug statement, 
activate, call plugin, deactive, debug statement, which is repeatitive. You can 
clean this up with aspectj annotations. With this, methods will just have an 
annotation and plugin call. Check GraphTransactionInterceptor for reference


- Shwetha


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


On July 5, 2016, 1:24 a.m., Madhan Neethiraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49585/
> -----------------------------------------------------------
> 
> (Updated July 5, 2016, 1:24 a.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-987
>     https://issues.apache.org/jira/browse/ATLAS-987
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Dependent libraries of Atlas hooks are loaded in a separate class loader, 
> there by making these libraries not visibile to components.
> 
> Here is the summary of the changes:
>   - current contents of atlas/hook/<component> directiries, including the 
> hook class implementation, are moved under 
> atlas/hook/<component>/atlas-<component>-plugin-impl directory
>   - a new component specific shim library, that includes the hook class 
> registered with the component, is placed in directory atlas/hook/<component>
>   - the hook class in the shim library loads all files in 
> atlas-<component>-plugin-impl directory in a classloader and forwards all the 
> calls on the shim class to the real implementation class
> 
> This implementation is same as the one used in Apache Ranger - more details 
> in RANGER-586.
> 
> 
> Diffs
> -----
> 
>   addons/falcon-bridge-shim/pom.xml PRE-CREATION 
>   
> addons/falcon-bridge-shim/src/main/java/org/apache/atlas/falcon/hook/AtlasService.java
>  PRE-CREATION 
>   addons/falcon-bridge/pom.xml d79dda9 
>   addons/hive-bridge-shim/pom.xml PRE-CREATION 
>   
> addons/hive-bridge-shim/src/main/java/org/apache/atlas/hive/hook/HiveHook.java
>  PRE-CREATION 
>   addons/hive-bridge/pom.xml ddefdc2 
>   addons/sqoop-bridge-shim/pom.xml PRE-CREATION 
>   
> addons/sqoop-bridge-shim/src/main/java/org/apache/atlas/sqoop/hook/SqoopHook.java
>  PRE-CREATION 
>   addons/sqoop-bridge/pom.xml c792945 
>   addons/storm-bridge-shim/pom.xml PRE-CREATION 
>   
> addons/storm-bridge-shim/src/main/java/org/apache/atlas/storm/hook/StormAtlasHook.java
>  PRE-CREATION 
>   addons/storm-bridge/pom.xml 9e8bf2f 
>   plugin-classloader/pom.xml PRE-CREATION 
>   
> plugin-classloader/src/main/java/org/apache/atlas/plugin/classloader/AtlasPluginClassLoader.java
>  PRE-CREATION 
>   
> plugin-classloader/src/main/java/org/apache/atlas/plugin/classloader/AtlasPluginClassLoaderUtil.java
>  PRE-CREATION 
>   pom.xml f119525 
> 
> Diff: https://reviews.apache.org/r/49585/diff/
> 
> 
> Testing
> -------
> 
> Verified that existing tests pass
> Verified Hive hook successfully sends notification to Kafka, which in turn 
> was processed correctly by Atlas server
> 
> 
> Thanks,
> 
> Madhan Neethiraj
> 
>

Reply via email to