----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72698/#review221372 -----------------------------------------------------------
atlas-examples/pom.xml Lines 1 (patched) <https://reviews.apache.org/r/72698/#comment310262> Copyright header missing. Please copy from an existing pom.xml file in Atlas. atlas-examples/sample-app/pom.xml Lines 1 (patched) <https://reviews.apache.org/r/72698/#comment310263> Copyright header missing. Please copy from an existing pom.xml file in Atlas. atlas-examples/sample-app/pom.xml Lines 18 (patched) <https://reviews.apache.org/r/72698/#comment310258> 3.0.0-SNAPSHOT => ${project.version} - please update at #23, #29, #35 as well atlas-examples/sample-app/pom.xml Lines 41 (patched) <https://reviews.apache.org/r/72698/#comment310259> 2.9.4 => ${jackson.version} - please update #57, #63 as well atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/DiscoveryExample.java Lines 31 (patched) <https://reviews.apache.org/r/72698/#comment310260> Do atlasClient and tableEntity_us need to be static? I suggest to remove static, and mark these as final, as they are initialized only in the constructor. atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/DiscoveryExample.java Lines 39 (patched) <https://reviews.apache.org/r/72698/#comment310261> Consider replacing this method with a static constant: private static final String[] DSL_QUERIES = new String[] { "from DataSet", "from Process" }; atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/DiscoveryExample.java Lines 64 (patched) <https://reviews.apache.org/r/72698/#comment310264> tableEntity_us.getTypeName() is the query-string to quickSearch(). Its not clear from this usage. I suggest to: - remove tableEntity_us as member - update quickSearch() with an argument public void quickSearch(String searchString) { ... } atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 41 (patched) <https://reviews.apache.org/r/72698/#comment310265> DATABASE_ENTITY_NAME => DATABASE_NAME TABLE_ENTITY_NAME => TABLE_NAME PROCESS_ENTITY_NAME => PROCESS_NAME atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 47 (patched) <https://reviews.apache.org/r/72698/#comment310266> NAME => ATTR_NAME DESCRIPTION => ATTR_DESCRIPTION QUALIFIED_NAME => ATTR_QUALIFIED_NAME TIME_ID_COLUMN => ATTR_TIME_ID_COLUMN CUSTOMER_ID_COLUMN => ATTR_CUSTOMER_ID_COLUMN COMPANY_ID_COLUMN => ATTR_COMPANY_ID_COLUMN atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 58 (patched) <https://reviews.apache.org/r/72698/#comment310267> I suggest to remove static for following members: - atlasClient - dbEntity - tableEntityUS - ttableEntityCanada - loadProcess - atlasClient can be declared as final, as it is initialized only in the constructor atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 101 (patched) <https://reviews.apache.org/r/72698/#comment310268> deleteEntityByGuid() => deleteEntities() atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 109 (patched) <https://reviews.apache.org/r/72698/#comment310269> getCreatedTableEntity() => createTableEntity() atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 117 (patched) <https://reviews.apache.org/r/72698/#comment310270> getCreatedLoadProcess() => createLoadProcess() atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 213 (patched) <https://reviews.apache.org/r/72698/#comment310278> dbInstance => database atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 229 (patched) <https://reviews.apache.org/r/72698/#comment310271> protected => private createHiveTableInstance() => createHiveTable() databaseInstance => database atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 230 (patched) <https://reviews.apache.org/r/72698/#comment310274> tableInstance => table atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 231 (patched) <https://reviews.apache.org/r/72698/#comment310272> Is #231 necessary, given it is overwriten at #248? atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 238 (patched) <https://reviews.apache.org/r/72698/#comment310277> REFERENCEABLE_ATTRIBUTE_NAME should be qualifiedName of the table; this shouldn't be set to tableName. Consider: database.getAttribute(NAME) + "." + tableName + METADATA_NAMESPACE_SUFFIX atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 246 (patched) <https://reviews.apache.org/r/72698/#comment310273> #246 is duplicate of #239. atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 251 (patched) <https://reviews.apache.org/r/72698/#comment310279> This assumes that the first classification (classificationNames[0]) type has an attribute named 'tag'. This is not intutive. atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 253 (patched) <https://reviews.apache.org/r/72698/#comment310275> serde1Instance => serde1 atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java Lines 258 (patched) <https://reviews.apache.org/r/72698/#comment310276> serde2Instance => serde2 atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/SampleApp.java Lines 28 (patched) <https://reviews.apache.org/r/72698/#comment310282> Consider removing 'static' from 'atlasClient', and the methods that refer to 'atlasClient'. atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/SampleAppConstants.java Lines 1 (patched) <https://reviews.apache.org/r/72698/#comment310280> License text missing. atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/SampleAppConstants.java Lines 9 (patched) <https://reviews.apache.org/r/72698/#comment310283> prefix all types with "sample-": - sample-pii_Tag - sample-finance_Tag - sample-db_type - sample-process_type - sample-table_type - sample-column_type - sample-Table_DB - sample-Table_Columns atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 57 (patched) <https://reviews.apache.org/r/72698/#comment310285> - atlasClient and typesDef need not be static. Please review and remove 'static' atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 59 (patched) <https://reviews.apache.org/r/72698/#comment310286> TYPES => SAMPLEAPP_TYPES atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 91 (patched) <https://reviews.apache.org/r/72698/#comment310287> this.typesDef = batchCreateTypes(typesDef); atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 94 (patched) <https://reviews.apache.org/r/72698/#comment310289> getAllTypeDef() implies some return value. The implementation simply retrieves and prints type-defs. atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 108 (patched) <https://reviews.apache.org/r/72698/#comment310288> removeAllTypeDef() => removeTypeDefintions() - to be symetrical to createTypeDefinitions() at #65 atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 117 (patched) <https://reviews.apache.org/r/72698/#comment310290> Avoid usinig conststraints. Instead, use relationship-types to relate 2 entity types. Replace CONSTRAINT_TYPE_OWNED_REF with isContainer=true for one end of the relationship. atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 121 (patched) <https://reviews.apache.org/r/72698/#comment310291> null => Collections.singleton("DataSet") atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 125 (patched) <https://reviews.apache.org/r/72698/#comment310292> name, description and owner are attributes of Asset, which is super-type of DatatSet. Please remove these attributes from SampleAppConstants.DATABASE_TYPE. atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 140 (patched) <https://reviews.apache.org/r/72698/#comment310293> owner is an attribute of Asset, which is super-type of DatatSet. Please remove this attribute from SampleAppConstants.TABLE_TYPE. atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 158 (patched) <https://reviews.apache.org/r/72698/#comment310294> null => Collections.singleton("DataSet") atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 159 (patched) <https://reviews.apache.org/r/72698/#comment310295> name is an attribute of Asset, which is super-type of DatatSet. Please remove this attribute from SampleAppConstants.COLUMN_TYPE. atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 177 (patched) <https://reviews.apache.org/r/72698/#comment310296> createStructDef() => createSerDeStructDef() atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 179 (patched) <https://reviews.apache.org/r/72698/#comment310299> Replace hardcoded string, serdeType, with a constant: SampleAppConstants.STRUCT_TYPE_SERDE atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 184 (patched) <https://reviews.apache.org/r/72698/#comment310297> createEnumDef() => createTableTypeEnumDef() atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 185 (patched) <https://reviews.apache.org/r/72698/#comment310300> Replace hardcoded string, tableType, with a constant: SampleAppConstants.ENUM_TABLE_TYPE atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java Lines 209 (patched) <https://reviews.apache.org/r/72698/#comment310298> Replace hardcoded string, bmWithAllTypes, with a constant: SampleAppConstants.BUSINESSMETADATA_TYPE Same for #217 as well. atlas-examples/sample-app/src/main/resources/atlas-application.properties Lines 1 (patched) <https://reviews.apache.org/r/72698/#comment310281> Lincese text missing atlas-examples/sample-app/src/main/resources/atlas-application.properties Lines 10 (patched) <https://reviews.apache.org/r/72698/#comment310284> Does the sample use Kafka notifications? If not, I suggest to remove these configurations. - Madhan Neethiraj On July 23, 2020, 6:10 p.m., Jyoti Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72698/ > ----------------------------------------------------------- > > (Updated July 23, 2020, 6:10 p.m.) > > > Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath > Subramanian, and Sidharth Mishra. > > > Bugs: ATLAS-3875 > https://issues.apache.org/jira/browse/ATLAS-3875 > > > Repository: atlas > > > Description > ------- > > Using this project users can get an idea as how to integrate with Atlas using > AtlasCleint. This helps the user to understand the basic rest functionality > of Atlas such as > > - EntityRest > - TypeDefRest > - DiscoveryRest > - LineageRest > - GlossaryRest > > > Diffs > ----- > > atlas-examples/pom.xml PRE-CREATION > atlas-examples/sample-app/README.md PRE-CREATION > atlas-examples/sample-app/pom.xml PRE-CREATION > > atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/DiscoveryExample.java > PRE-CREATION > > atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/EntityExample.java > PRE-CREATION > > atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/GlossaryExample.java > PRE-CREATION > > atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/LineageExample.java > PRE-CREATION > > atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/SampleApp.java > PRE-CREATION > > atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/SampleAppConstants.java > PRE-CREATION > > atlas-examples/sample-app/src/main/java/org/apache/atlas/examples/sampleapp/TypeDefExample.java > PRE-CREATION > atlas-examples/sample-app/src/main/resources/atlas-application.properties > PRE-CREATION > pom.xml 5e0442ae5 > > > Diff: https://reviews.apache.org/r/72698/diff/3/ > > > Testing > ------- > > > Thanks, > > Jyoti Singh > >