----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73929/#review225028 -----------------------------------------------------------
distro/src/conf/atlas-application.properties Lines 282 (patched) <https://reviews.apache.org/r/73929/#comment313828> Please make it - "Skip check for the same attribute name in Parent type and Child type" intg/src/main/java/org/apache/atlas/ThreadContext.java Lines 25 (patched) <https://reviews.apache.org/r/73929/#comment313839> Even this can be removed by using thread name check. For this we can use Thread.setName and getName to see if it's the main thread. intg/src/main/java/org/apache/atlas/ThreadContext.java Lines 26-27 (patched) <https://reviews.apache.org/r/73929/#comment313838> We don't need to keep the type names as thread local. Please check my later comment to get more details. intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java Line 101 (original), 105 (patched) <https://reviews.apache.org/r/73929/#comment313835> getTypeHierarchyInfo is a private function and classificationDef is a member variable. Why do we need to pass this as an argument? intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java Line 528 (original), 532 (patched) <https://reviews.apache.org/r/73929/#comment313836> Why do we need to pass typeDefTobeCreatedOrUpdated as an argument? Can we use classificationDef.getName() instead. intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java Line 132 (original), 133 (patched) <https://reviews.apache.org/r/73929/#comment313837> Why do we need to pass entityDef.getName() to all the respective private function calls as this is a private variable and can be accessed directly. intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java Lines 890 (patched) <https://reviews.apache.org/r/73929/#comment313843> As I mentioned for AtlasClassificationType, can we simply here as well to - if (attributeToEntityNameMap.containsKey(attributeName) && !attributeToEntityNameMap.get(attributeName).equals(entityDef.getName())) { if (Thread.currentThread().getName().equalsIgnoreCase(STARTUP_THREAD) && skipCheckForParentChildAttributeName) { LOG.warn(AtlasErrorCode.ATTRIBUTE_NAME_ALREADY_EXISTS_IN_ANOTHER_PARENT_TYPE.getFormattedErrorMessage(entityDef.getName(), attributeName, attributeToEntityNameMap.get(attributeName))); } else { throw new AtlasBaseException(AtlasErrorCode.ATTRIBUTE_NAME_ALREADY_EXISTS_IN_ANOTHER_PARENT_TYPE, entityDef.getName(), attributeName, attributeToEntityNameMap.get(attributeName)); } } intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java Lines 891 (patched) <https://reviews.apache.org/r/73929/#comment313840> Please check if we can simplify this to : if (attributeToClassificationNameMap.containsKey(attributeName) && !attributeToClassificationNameMap.get(attributeName).equals(classificationDef.getName())) { if (Thread.currentThread().getName().equalsIgnoreCase(STARTUP_THREAD) && skipCheckForParentChildAttributeName)) { LOG.warn(AtlasErrorCode.ATTRIBUTE_NAME_ALREADY_EXISTS_IN_ANOTHER_PARENT_TYPE.getFormattedErrorMessage(classificationDef.getName(), attributeName, attributeToClassificationNameMap.get(attributeName))); } else { throw new AtlasBaseException(AtlasErrorCode.ATTRIBUTE_NAME_ALREADY_EXISTS_IN_ANOTHER_PARENT_TYPE, classificationDef.getName(), attributeName, attributeToClassificationNameMap.get(attributeName)); } } P.S. STARTUP_THREAD can be set to "main" and setName at STARTUP_THREAD at Atlas.java intg/src/main/java/org/apache/atlas/type/AtlasStructType.java Line 723 (original) <https://reviews.apache.org/r/73929/#comment313842> I believe this was a redundant code and this is been taken care by https://issues.apache.org/jira/browse/ATLAS-4522 and https://issues.apache.org/jira/browse/ATLAS-4668 fix intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java Lines 595 (patched) <https://reviews.apache.org/r/73929/#comment313841> As I mentioned we can remove these setter as we don't need the same. - Sidharth Mishra On Dec. 16, 2022, 6:41 a.m., Mandar Ambawane wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73929/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2022, 6:41 a.m.) > > > Review request for atlas, Ashutosh Mestry, Jayendra Parab, Madhan Neethiraj, > Paresh Devalia, Pinal Shah, Radhika Kundam, Sarath Subramanian, Sheetal Shah, > and Sidharth Mishra. > > > Bugs: ATLAS-4576 > https://issues.apache.org/jira/browse/ATLAS-4576 > > > Repository: atlas > > > Description > ------- > > Currently we have provided 2 checks for the Attribute Name: > > https://issues.apache.org/jira/browse/ATLAS-3872 > Restrict typedef creation when a child type attribute conflicts with parent > type attribute of same name > > https://issues.apache.org/jira/browse/ATLAS-4522 > Updating typedef with new supertype should be allowed only if attributes are > unique compared to other existing supertypes > > But in the earlier versions these checks were not there. So there may be a > chance of Atlas environment having such data where these checks can cause > problems at the time: > 1. Atlas Startup. > 2. Creating new typedefs > > This patch handles these scenarios and provides backward compatibility for > these 2 changes mentioned. > > > Diffs > ----- > > distro/src/conf/atlas-application.properties c92c619c9 > intg/src/main/java/org/apache/atlas/ThreadContext.java PRE-CREATION > intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java > 17422cf95 > intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java 76bee36f2 > intg/src/main/java/org/apache/atlas/type/AtlasStructType.java 21ce23657 > intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 0c05bb399 > intg/src/test/java/org/apache/atlas/type/TestAtlasClassificationType.java > b6055fa30 > intg/src/test/java/org/apache/atlas/type/TestAtlasEntityType.java 742970390 > webapp/src/main/java/org/apache/atlas/Atlas.java 7cf6e3eab > > > Diff: https://reviews.apache.org/r/73929/diff/5/ > > > Testing > ------- > > PreCommit: > https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1084/consoleFull > > PreCommit: > https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1293/consoleFull > > > Thanks, > > Mandar Ambawane > >
