> On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/README.md > > Lines 132 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983359#file1983359line132> > > > > I suggest not using I in the readme.
deleted all "I"s > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/README.md > > Lines 141 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983359#file1983359line141> > > > > I suggest removing Kees name for the comment. removed all "Kees"s > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/ColumnContextEvent.java > > Lines 21 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983362#file1983362line21> > > > > why can we leave this blank. I htink this needs to have a value. deleted, thanks! > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/Connection.java > > Lines 15 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983364#file1983364line15> > > > > why does this have xml tags? all the codes in org.apache.atlas.omas.informationview.events will be eventually developed by Ruxandra. So these code will show up as a dependency(informationview) for virtualiser. I will talk with Ruxandra and make a change. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/omas/informationview/events/Connection.java > > Lines 20 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983364#file1983364line20> > > > > 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. It is OCF Connection here, just for convenience, we name it connnection here. Maybe IVConnection is better? Note: all the codes in org.apache.atlas.omas.informationview.events will be eventually developed by Ruxandra. So these code will show up as a dependency(informationview) for virtualiser. I will talk with Ruxandra and make a change. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/KafkaVirtualiser.java > > Lines 20 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983367#file1983367line20> > > > > 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. since we are both not commiter and can not be added to the main pom.xml in Atlas project. I deleted this for now. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/KafkaVirtualiser.java > > Lines 49 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983367#file1983367line49> > > > > I am not sure how this will run - shouldnt it run in a thread? for now we will run virtualiser as a jar and run separately. I think eventually it will run as a thread. I will keep this issure open until i change the code and run virtualiser as a thread. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/KafkaVirtualiser.java > > Lines 67 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983367#file1983367line67> > > > > Do you want to delete this function - you could leave it in for testing. it will depend on how we run Virtualiser later as a sperate thread or a seprate jar. I will keep this issue open. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasAPICalls.java > > Lines 49 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983368#file1983368line49> > > > > I was not expecting any Atlas specific class names in the virtualiser; > > I would expect it to only use omas or omrs APIs. deleted all related Atlas parts for Virtualiser. It is not Virtualiser's task. Virtualiser sends out business and technical view, Information View OMAS will update Atlas. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasAPICalls.java > > Lines 87 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983368#file1983368line87> > > > > I suggest using Jackson. The atlas code base has made changes to use > > one technology around parse json to java - which is jackson. deleted all related Atlas parts for Virtualiser. It is not Virtualiser's task. Virtualiser sends out business and technical view, Information View OMAS will update Atlas. and also Virtualiser and Information View OMAS use Jackson now. No worries. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/atlas/AtlasAPICalls.java > > Lines 97 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983368#file1983368line97> > > > > We should deal with the error properly. Logging it. should we gobble > > the exception? deleted all related Atlas parts for Virtualiser. It is not Virtualiser's task. Virtualiser sends out business and technical view, Information View OMAS will update Atlas. and also Virtualiser and Information View OMAS use Jackson now. No worries. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/ffdc/VirtualiserErrorCode.java > > Lines 52 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983377#file1983377line52> > > > > I am not sure who would be making this action of checking newInstance. > > Is this a code change or something a user could change? the code change, this exception pops up when the developer use "Class.forName(PropertiesHelper.properties.getProperty("derby_driver")).newInstance();" when they want to connect to Gaian. they might use the wrong driver name. So the developer who set up the property has to check newInstance(). > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/ffdc/VirtualiserErrorCode.java > > Lines 72 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983377#file1983377line72> > > > > what is the users valid information? userId and password. they will be used to connect to Gaian. Added the detailed info in this errorcode. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/kafka/KafkaVirtualiserConsumer.java > > Lines 46 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983382#file1983382line46> > > > > shouldnt we use consumer groups as more than one consumer could be > > listening on the topic. Good idea. I will keep this issue open until I change the code properly > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/kafka/KafkaVirtualiserConsumer.java > > Lines 120 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983382#file1983382line120> > > > > delete deleted > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java > > Lines 41 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983386#file1983386line41> > > > > I do not think you need to have this variable. As an empty view > > indicated we have nothing mapped. deleted, and change it to whether views==null now > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java > > Lines 47 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983386#file1983386line47> > > > > hich typo fixed, thanks! > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java > > Lines 54 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983386#file1983386line54> > > > > 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. agree, change it to null. and added assertNotNull(views) in test. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java > > Lines 58 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983386#file1983386line58> > > > > why dont we test if views == null - do we need noneMapped variable agree, changed to if(views==null), thanks > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java > > Lines 63 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983386#file1983386line63> > > > > log the exception log the error and also add log.debug for debugging > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java > > Lines 109 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983386#file1983386line109> > > > > delete deleted. thanks! > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/java/org/apache/atlas/virtualiser/views/ViewsConstructor.java > > Lines 181 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983386#file1983386line181> > > > > buinsess typo fixed, thanks! > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/resources/input.json > > Lines 11 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983391#file1983391line11> > > > > 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. this is json file is generated from IGC. Deleted.Thanks! > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/resources/virtualiser.properties > > Lines 9 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983394#file1983394line9> > > > > This file should be a sample as it has specific values in, rather than > > as part of the main code. this will be the settings for using Virtualiser. So it has to be there until we have a central properties file where I can move all these properties to the central file. > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/main/resources/virtualiser.properties > > Lines 56 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983394#file1983394line56> > > > > We hsould not be using the atlas API and there should not be ing urls > > in Atlas. deleted > On March 20, 2018, 7:35 p.m., David Radley wrote: > > virtualiser/src/test/java/org/apache/atlas/virtualiser/JsonReadHelper.java > > Lines 33 (patched) > > <https://reviews.apache.org/r/66174/diff/1/?file=1983395#file1983395line33> > > > > 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. put it after the finally section - Yao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66174/#review199553 ----------------------------------------------------------- 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 > >
