----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66174/#review199553 -----------------------------------------------------------
virtualiser/README.md Lines 20 (patched) <https://reviews.apache.org/r/66174/#comment279852> maily - spelling error we need to explain what Gaian is, where to get it. virtualiser/README.md Lines 22 (patched) <https://reviews.apache.org/r/66174/#comment279851> funcitons - spelling error virtualiser/README.md Lines 23 (patched) <https://reviews.apache.org/r/66174/#comment279853> I suggest not using file - use payload. virtualiser/README.md Lines 31 (patched) <https://reviews.apache.org/r/66174/#comment279850> needs to remove these characters virtualiser/README.md Lines 94 (patched) <https://reviews.apache.org/r/66174/#comment279854> I am not sure what this line means virtualiser/README.md Lines 132 (patched) <https://reviews.apache.org/r/66174/#comment279856> I suggest not using I in the readme. virtualiser/README.md Lines 141 (patched) <https://reviews.apache.org/r/66174/#comment279855> I suggest removing Kees name for the comment. virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/ColumnContextEvent.java Lines 21 (patched) <https://reviews.apache.org/r/66174/#comment279857> why can we leave this blank. I htink this needs to have a value. virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/Connection.java Lines 15 (patched) <https://reviews.apache.org/r/66174/#comment279859> why does this have xml tags? virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/Connection.java Lines 20 (patched) <https://reviews.apache.org/r/66174/#comment279858> I would think that a connection would fit in the OCF framework. This class has a gaianNodeName, so the name of the class should probably include gaian. virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/DerivedColumnDetail.java Lines 16 (patched) <https://reviews.apache.org/r/66174/#comment279860> why is this always but InformationViewEvent is not null? virtualiser/src/main/java/org/apache/atlas/virtualiser/KafkaVirtualiser.java Lines 20 (patched) <https://reviews.apache.org/r/66174/#comment279845> It is not usual to add the authors for creation and updates at the top of the files. If you are going to do this, I think the convention is to use @author tags. virtualiser/src/main/java/org/apache/atlas/virtualiser/KafkaVirtualiser.java Lines 49 (patched) <https://reviews.apache.org/r/66174/#comment279838> I am not sure how this will run - shouldnt it run in a thread? virtualiser/src/main/java/org/apache/atlas/virtualiser/KafkaVirtualiser.java Lines 67 (patched) <https://reviews.apache.org/r/66174/#comment279846> Do you want to delete this function - you could leave it in for testing. virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasAPICalls.java Lines 49 (patched) <https://reviews.apache.org/r/66174/#comment279837> I was not expecting any Atlas specific class names in the virtualiser; I would expect it to only use omas or omrs APIs. virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasAPICalls.java Lines 87 (patched) <https://reviews.apache.org/r/66174/#comment279848> I suggest using Jackson. The atlas code base has made changes to use one technology around parse json to java - which is jackson. virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasAPICalls.java Lines 92 (patched) <https://reviews.apache.org/r/66174/#comment279847> This logic looks strange, you have a for loop and then you get the first entity and then return. The code gets the first element - what about the otehr entities in the response; they seem to be ignored. virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasAPICalls.java Lines 97 (patched) <https://reviews.apache.org/r/66174/#comment279849> We should deal with the error properly. Logging it. should we gobble the exception? virtualiser/src/main/java/org/apache/atlas/virtualiser/ffdc/VirtualiserErrorCode.java Lines 52 (patched) <https://reviews.apache.org/r/66174/#comment279861> I am not sure who would be making this action of checking newInstance. Is this a code change or something a user could change? virtualiser/src/main/java/org/apache/atlas/virtualiser/ffdc/VirtualiserErrorCode.java Lines 72 (patched) <https://reviews.apache.org/r/66174/#comment279862> what is the users valid information? virtualiser/src/main/java/org/apache/atlas/virtualiser/kafka/KafkaVirtualiserConsumer.java Lines 46 (patched) <https://reviews.apache.org/r/66174/#comment279885> shouldnt we use consumer groups as more than one consumer could be listening on the topic. virtualiser/src/main/java/org/apache/atlas/virtualiser/kafka/KafkaVirtualiserConsumer.java Lines 120 (patched) <https://reviews.apache.org/r/66174/#comment279886> delete virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java Lines 41 (patched) <https://reviews.apache.org/r/66174/#comment279873> I do not think you need to have this variable. As an empty view indicated we have nothing mapped. virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java Lines 47 (patched) <https://reviews.apache.org/r/66174/#comment279874> hich typo virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java Lines 54 (patched) <https://reviews.apache.org/r/66174/#comment279876> No views could mean this method returns null. If there is an exception then views will be ("",""}. Unless there is a reason I suggest views are null or have values. virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java Lines 58 (patched) <https://reviews.apache.org/r/66174/#comment279877> why dont we test if views == null - do we need noneMapped variable virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java Lines 63 (patched) <https://reviews.apache.org/r/66174/#comment279875> log the exception virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java Lines 101 (patched) <https://reviews.apache.org/r/66174/#comment279879> delete virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java Lines 109 (patched) <https://reviews.apache.org/r/66174/#comment279880> delete virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java Lines 181 (patched) <https://reviews.apache.org/r/66174/#comment279872> buinsess typo virtualiser/src/main/resources/input.json Lines 11 (patched) <https://reviews.apache.org/r/66174/#comment279871> I am unsure why we have specifc guid values in the main code. I would think I would see guid values in the test suite but not in the main java. virtualiser/src/main/resources/virtualiser.properties Lines 9 (patched) <https://reviews.apache.org/r/66174/#comment279881> This file should be a sample as it has specific values in, rather than as part of the main code. virtualiser/src/main/resources/virtualiser.properties Lines 56 (patched) <https://reviews.apache.org/r/66174/#comment279882> We hsould not be using the atlas API and there should not be ing urls in Atlas. virtualiser/src/test/java/org/apache/atlas/virtualiser/JsonReadHelper.java Lines 21 (patched) <https://reviews.apache.org/r/66174/#comment279883> delete virtualiser/src/test/java/org/apache/atlas/virtualiser/JsonReadHelper.java Lines 33 (patched) <https://reviews.apache.org/r/66174/#comment279884> It is not usual to put returns in finally sections. I suggest putting it at the end of the try or after the finally section. - David Radley On March 20, 2018, 2:25 p.m., Yao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66174/ > ----------------------------------------------------------- > > (Updated March 20, 2018, 2:25 p.m.) > > > Review request for atlas, Kees van de Fliert, Mandy Chessell, and Maryna > Strelchuk. > > > Repository: atlas > > > Description > ------- > > Virtualiser has three main funcitons: > 1. listen to Information View OMAS (information-view-out-topic) and retrieve > the json file; > 2. update business logic table and technical logic table, send the updates to > Gaian; > 3. create business view json file and technical view json file, notify > Information View OMAS the update(information-view-in-topic). > > The JIRA can be found https://issues.apache.org/jira/browse/ATLAS-1831 > > > Diffs > ----- > > virtualiser/README.md PRE-CREATION > virtualiser/pom.xml PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/BusinessTerm.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/ColumnContextEvent.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/ColumnDetails.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/Connection.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/DerivedColumnDetail.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/InformationViewEvent.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/KafkaVirtualiser.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasAPICalls.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasAttributes.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasCriterion.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasEntity.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasEntityFilters.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasSearchParameters.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasSearchResponse.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/ffdc/VirtualiserCheckedException.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/ffdc/VirtualiserCheckedExceptionBase.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/ffdc/VirtualiserErrorCode.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/ffdc/VirtualiserRuntimeException.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/gaian/GaianQueryConstructor.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/gaian/LogicTable.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/gaian/MappedColumn.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/kafka/KafkaVirtualiserConsumer.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/kafka/KafkaVirtualiserProducer.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/util/ExecuteQueryUtil.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/util/PropertiesHelper.java > PRE-CREATION > > virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java > PRE-CREATION > virtualiser/src/main/resources/business.json PRE-CREATION > virtualiser/src/main/resources/createEntityinAtlas.json PRE-CREATION > virtualiser/src/main/resources/delete.json PRE-CREATION > virtualiser/src/main/resources/fullAssign.json PRE-CREATION > virtualiser/src/main/resources/input.json PRE-CREATION > virtualiser/src/main/resources/technical.json PRE-CREATION > virtualiser/src/main/resources/testIn.json PRE-CREATION > virtualiser/src/main/resources/virtualiser.properties PRE-CREATION > virtualiser/src/test/java/org/apache/atlas/virtualiser/JsonReadHelper.java > PRE-CREATION > > virtualiser/src/test/java/org/apache/atlas/virtualiser/KafkaVirtualiserProducerForTest.java > PRE-CREATION > > virtualiser/src/test/java/org/apache/atlas/virtualiser/KafkaVirtualiserTest.java > PRE-CREATION > > virtualiser/src/test/java/org/apache/atlas/virtualiser/gaian/GaianQueryConstructorTest.java > PRE-CREATION > > virtualiser/src/test/java/org/apache/atlas/virtualiser/views/ViewsConstructorTest.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/66174/diff/1/ > > > Testing > ------- > > see test folder > > > Thanks, > > Yao Li > >
