----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71649/#review218727 -----------------------------------------------------------
I don't see namespace attribuites indexed. This should be indexed. Check GraphBackSearchIndexer.addIndexForType() method intg/src/main/java/org/apache/atlas/model/typedef/AtlasNamespaceDef.java Lines 174 (patched) <https://reviews.apache.org/r/71649/#comment306592> removed unused constructors in line 174, 179, 193 and 203 intg/src/main/java/org/apache/atlas/model/typedef/AtlasNamespaceDef.java Lines 162 (patched) <https://reviews.apache.org/r/71649/#comment306600> 'maxStrLength' and 'validPattern' are string specific attribute options. Consider adding a enum - namespaceAttributeOptions {MAX_STRING_LENGTH, VALID_PATTERN} and use 'options' map in AtlasAttributeDef to set the namespace attribute options. This will be easier to extend for any int, date options as well. intg/src/main/java/org/apache/atlas/store/AtlasTypeDefStore.java Lines 83 (patched) <https://reviews.apache.org/r/71649/#comment306617> add REST endpoints in TypesREST for: /namespacedef/name/{name} /namespacedef/guid/{guid} repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 22 (patched) <https://reviews.apache.org/r/71649/#comment306601> nit: remove unused imports in line 22,42 repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 59 (patched) <https://reviews.apache.org/r/71649/#comment306604> validateType() checks for invalid types only for structDef attruibutes, add check in validateType() to check for namespace attributes as well. repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 133 (patched) <https://reviews.apache.org/r/71649/#comment306605> is this line needed? please review. repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 226 (patched) <https://reviews.apache.org/r/71649/#comment306613> "update classification-def " => "update namespace-def " repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 260 (patched) <https://reviews.apache.org/r/71649/#comment306614> "delete struct-def " => "delete namespace-def " repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 297 (patched) <https://reviews.apache.org/r/71649/#comment306610> AtlasBaseException is never thrown repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 349 (patched) <https://reviews.apache.org/r/71649/#comment306602> 'if' condition in line 349 and 353 is duplicate. please review. repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 357 (patched) <https://reviews.apache.org/r/71649/#comment306603> check for null as well for 'applicableEntityTypes'. => CollectionUtils.isNotEmpty(attributeDef.getApplicableEntityTypes()) repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 382 (patched) <https://reviews.apache.org/r/71649/#comment306612> => CollectionUtils.isNotEmpty(attributeDef.getApplicableEntityTypes()) repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 470 (patched) <https://reviews.apache.org/r/71649/#comment306606> refactor to: if (!currAttrNames.containsAll(attrNames)) { //throw exception } repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 496 (patched) <https://reviews.apache.org/r/71649/#comment306607> check for null attributeDef.getApplicableEntityTypes() as well ? repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 515 (patched) <https://reviews.apache.org/r/71649/#comment306608> => if(!updatedApplicableEntityTypes.containsAll(existingAttribute.getApplicableEntityTypes())) { // throw exception } repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 526 (patched) <https://reviews.apache.org/r/71649/#comment306609> set using encoded property key here: AtlasGraphUtilsV2.encodePropertyKey(propertyKey); repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java Line 694 (original), 694 (patched) <https://reviews.apache.org/r/71649/#comment306616> contains only whitespace changes. revert changes to this file - Sarath Subramanian On Nov. 20, 2019, 7:03 p.m., Aadarsh Jajodia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71649/ > ----------------------------------------------------------- > > (Updated Nov. 20, 2019, 7:03 p.m.) > > > Review request for atlas, Ashutosh Mestry, Sridhar K, Le Ma, Madhan > Neethiraj, and Sarath Subramanian. > > > Bugs: ATLAS-3486 > https://issues.apache.org/jira/browse/ATLAS-3486 > > > Repository: atlas > > > Description > ------- > > This change is the first part of the bigger task defined as part of > ATLAS-3485. > This adds the data model needed for supporting namespaces and namespace > attributes and also updates the type registry to include the applicable > namespace attributes for every entity type > > > Diffs > ----- > > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java > 7a2aae2e9ae8174c5309164f3e41c940cbf3ddf8 > intg/src/main/java/org/apache/atlas/model/TypeCategory.java > f06f64f450f407e3f9a0e742726ff4dd12ccc695 > intg/src/main/java/org/apache/atlas/model/typedef/AtlasNamespaceDef.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java > bb7ead0f9f8bab3094eb82e9e286dd58e8a6e3de > intg/src/main/java/org/apache/atlas/model/typedef/AtlasTypesDef.java > 3634fdfd313639eb97b3c4698e091487b0e44a80 > intg/src/main/java/org/apache/atlas/store/AtlasTypeDefStore.java > 4ee68a936f99bb4c819b5335da2cc8bf7d539397 > intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java > 557ef74a95c2a939b4b89cd1db8fa4c73d52dd51 > intg/src/main/java/org/apache/atlas/type/AtlasNamespaceType.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java > b071dc9d664cee9e1ffc54726ffbf15f4f602d30 > intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java > 0883d54f490e22c6510e6fc0cb804b87713a7ecb > intg/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java > dba2d88146eff314191ae6bb24ad7337b0ea10ae > intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java > 02613b5f7250b14324ed294c22de079b74d55b08 > intg/src/test/java/org/apache/atlas/TestUtilsV2.java > ff79994c519702e90b2e478d00cae0008889f956 > > intg/src/test/java/org/apache/atlas/model/typedef/TestAtlasNamespaceDef.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java > 2e2ab1a664171555c57560e1c0b4cbdbc20c0f6f > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasTypeDefGraphStoreV2.java > a5ccfb5b2055c88f596312f4033bc0034d3d165c > > repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java > 51dd16b8518c9a16088547f3e95c0ef401695895 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2Test.java > PRE-CREATION > webapp/src/main/java/org/apache/atlas/examples/QuickStartV2.java > 6cd0ee331b7ae24757b58e76ec47bf556106846a > > > Diff: https://reviews.apache.org/r/71649/diff/7/ > > > Testing > ------- > > Added unit tests > > > Thanks, > > Aadarsh Jajodia > >
