-----------------------------------------------------------
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
> 
>

Reply via email to