----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51896/#review149121 -----------------------------------------------------------
common/src/main/java/org/apache/atlas/model/AtlasAttributeDef.java (line 55) <https://reviews.apache.org/r/51896/#comment216635> Generally, please add javadoc that describes the parameters and their intent. common/src/main/java/org/apache/atlas/model/AtlasAttributeDef.java (line 58) <https://reviews.apache.org/r/51896/#comment216637> This seems redundant. Isn't it automatically optional if valuesMinCount = 0? common/src/main/java/org/apache/atlas/model/AtlasAttributeDef.java (line 61) <https://reviews.apache.org/r/51896/#comment216634> Why is this needed? It seems like if 'valuesMaxCount' > 1, it is automatically multi-valued. common/src/main/java/org/apache/atlas/model/AtlasAttributeDef.java (line 62) <https://reviews.apache.org/r/51896/#comment216633> Should valuesMinCount/valuesMaxCount be changed to lowerBound/upperBound? Those seem like more intuitive names. common/src/main/java/org/apache/atlas/model/AtlasAttributeDef.java (line 64) <https://reviews.apache.org/r/51896/#comment216636> How is this different from isUnique? common/src/main/java/org/apache/atlas/model/AtlasBaseModelObject.java (line 45) <https://reviews.apache.org/r/51896/#comment216638> Could you change these to a java enumeration? Most serialization frameworks have no trouble with enumerations. common/src/main/java/org/apache/atlas/model/AtlasBaseModelObject.java (line 47) <https://reviews.apache.org/r/51896/#comment216654> It's not clear what "disabled" is and how it is different from "deleted". I also tend to think that the status should be an internal attribute that is not exposed. It seems like by default, users should be able to assume that any object that comes back from a query has not been deleted. I know that is not currently the case though. The query behavior really should not depend on whether hard or soft delete is being used, though, it should be transparent. I would prefer to have a separate set of apis for managing the entity status where you can query for deleted objects and change their state back to ACTIVE if desired. common/src/main/java/org/apache/atlas/model/AtlasBuiltInDatatypes.java (line 22) <https://reviews.apache.org/r/51896/#comment216640> It seems like constants like this should be declared once in the code and referred to everyplace else. For example, AtlasClient.DataSet, AtlasClient.Process, BooleanType.name (which could be made public). common/src/main/java/org/apache/atlas/model/AtlasBuiltInDatatypes.java (line 33) <https://reviews.apache.org/r/51896/#comment216639> I'm not sure what this is. Is it a data type? common/src/main/java/org/apache/atlas/model/AtlasBuiltInDatatypes.java (line 34) <https://reviews.apache.org/r/51896/#comment216641> I think you are missing a few built-in types like dataset, etc. The process type name also starts with a capital P. - Jeff Hagelberg On Sept. 19, 2016, 6:51 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51896/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2016, 6:51 p.m.) > > > Review request for atlas. > > > Bugs: ATLAS-1171 > https://issues.apache.org/jira/browse/ATLAS-1171 > > > Repository: atlas > > > Description > ------- > > first-cut API for review > > > Diffs > ----- > > common/pom.xml e3b6465 > common/src/main/java/org/apache/atlas/api/AtlasEntities.java PRE-CREATION > common/src/main/java/org/apache/atlas/api/AtlasTypes.java PRE-CREATION > common/src/main/java/org/apache/atlas/api/SearchFilter.java PRE-CREATION > common/src/main/java/org/apache/atlas/model/instances/AtlasEntity.java > PRE-CREATION > common/src/main/java/org/apache/atlas/model/instances/AtlasObjectId.java > PRE-CREATION > common/src/main/java/org/apache/atlas/model/instances/AtlasProcess.java > PRE-CREATION > common/src/main/java/org/apache/atlas/model/instances/AtlasStruct.java > PRE-CREATION > common/src/main/java/org/apache/atlas/model/instances/AtlasTrait.java > PRE-CREATION > common/src/main/java/org/apache/atlas/model/types/AtlasBaseTypeDef.java > PRE-CREATION > common/src/main/java/org/apache/atlas/model/types/AtlasEntityDef.java > PRE-CREATION > common/src/main/java/org/apache/atlas/model/types/AtlasEnumDef.java > PRE-CREATION > common/src/main/java/org/apache/atlas/model/types/AtlasStructDef.java > PRE-CREATION > common/src/main/java/org/apache/atlas/model/types/AtlasTraitDef.java > PRE-CREATION > common/src/main/java/org/apache/atlas/model/types/AtlasTypeId.java > PRE-CREATION > common/src/main/java/org/apache/atlas/typesystem/AtlasArrayType.java > PRE-CREATION > common/src/main/java/org/apache/atlas/typesystem/AtlasBuiltInTypes.java > PRE-CREATION > common/src/main/java/org/apache/atlas/typesystem/AtlasDataType.java > PRE-CREATION > common/src/main/java/org/apache/atlas/typesystem/AtlasEntityType.java > PRE-CREATION > common/src/main/java/org/apache/atlas/typesystem/AtlasEnumType.java > PRE-CREATION > common/src/main/java/org/apache/atlas/typesystem/AtlasMapType.java > PRE-CREATION > common/src/main/java/org/apache/atlas/typesystem/AtlasStructType.java > PRE-CREATION > common/src/main/java/org/apache/atlas/typesystem/AtlasTraitType.java > PRE-CREATION > common/src/main/java/org/apache/atlas/typesystem/AtlasTypeRegistry.java > PRE-CREATION > common/src/test/java/org/apache/atlas/model/ModelTestUtil.java PRE-CREATION > common/src/test/java/org/apache/atlas/model/instances/TestAtlasEntity.java > PRE-CREATION > common/src/test/java/org/apache/atlas/model/types/TestAtlasEntityDef.java > PRE-CREATION > common/src/test/java/org/apache/atlas/model/types/TestAtlasEnumDef.java > PRE-CREATION > common/src/test/java/org/apache/atlas/model/types/TestAtlasStructDef.java > PRE-CREATION > common/src/test/java/org/apache/atlas/model/types/TestAtlasTraitDef.java > PRE-CREATION > pom.xml ac5b042 > > Diff: https://reviews.apache.org/r/51896/diff/ > > > Testing > ------- > > > Thanks, > > Madhan Neethiraj > >
