> On Sept. 19, 2016, 11:13 p.m., Jeff Hagelberg wrote: > > common/src/main/java/org/apache/atlas/model/AtlasAttributeDef.java, line 58 > > <https://reviews.apache.org/r/51896/diff/1/?file=1498599#file1498599line58> > > > > This seems redundant. Isn't it automatically optional if > > valuesMinCount = 0?
Following attributes replace current use of type 'Multiplicity' - since this type does not seem very intutive. I think use of simply named attributes make it intutive. - isOptional, isMultiValued, valuesMinCount, valuesMaxCount, areValuesUnique > On Sept. 19, 2016, 11:13 p.m., Jeff Hagelberg wrote: > > common/src/main/java/org/apache/atlas/model/AtlasAttributeDef.java, line 61 > > <https://reviews.apache.org/r/51896/diff/1/?file=1498599#file1498599line61> > > > > Why is this needed? It seems like if 'valuesMaxCount' > 1, it is > > automatically multi-valued. Please see my earlier commit. I think the use of explicit attributes like isMultiValued make it easier to read/understand. > On Sept. 19, 2016, 11:13 p.m., Jeff Hagelberg wrote: > > common/src/main/java/org/apache/atlas/model/AtlasAttributeDef.java, line 62 > > <https://reviews.apache.org/r/51896/diff/1/?file=1498599#file1498599line62> > > > > Should valuesMinCount/valuesMaxCount be changed to > > lowerBound/upperBound? Those seem like more intuitive names. "bound" might mean the value of the attribute, ranther than the number of values in a multi-value attribute. Hence the names valuesMinCount and valuesMaxCount. > On Sept. 19, 2016, 11:13 p.m., Jeff Hagelberg wrote: > > common/src/main/java/org/apache/atlas/model/AtlasAttributeDef.java, line 64 > > <https://reviews.apache.org/r/51896/diff/1/?file=1498599#file1498599line64> > > > > How is this different from isUnique? isUnique is to specify if the attribute value should be unique across all entities. areValuesUnique is specific to multi-values attributes (isMultiValued=true), to specify if the values of an attribute instance should be unique. > On Sept. 19, 2016, 11:13 p.m., Jeff Hagelberg wrote: > > common/src/main/java/org/apache/atlas/model/AtlasBaseModelObject.java, line > > 47 > > <https://reviews.apache.org/r/51896/diff/1/?file=1498600#file1498600line47> > > > > 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. - removed "disabled". - "status" field will be an internal field, which will only be set in create/delete calls. If explicit status change is necessary, we can add separate APIs. > On Sept. 19, 2016, 11:13 p.m., Jeff Hagelberg wrote: > > common/src/main/java/org/apache/atlas/model/AtlasBuiltInDatatypes.java, > > line 22 > > <https://reviews.apache.org/r/51896/diff/1/?file=1498601#file1498601line22> > > > > 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). - moved ATLAS_TYPE_* constants to AtlasBaseTypeDefs - added types Process, DataSet, Asset, Infrastructure, Referenceable to this built-in types. Yet to add classes for many of these though. > On Sept. 19, 2016, 11:13 p.m., Jeff Hagelberg wrote: > > common/src/main/java/org/apache/atlas/model/AtlasBuiltInDatatypes.java, > > line 33 > > <https://reviews.apache.org/r/51896/diff/1/?file=1498601#file1498601line33> > > > > I'm not sure what this is. Is it a data type? This type is used to reference to another entity. I think it might be easier to continue with the current approach of directly using the entity-type name as the dataTypeName - like hive_table having 'dbName' attribute of type 'hive_database'. I will remove 'objectid' type shortly. - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51896/#review149121 ----------------------------------------------------------- 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 > >
