> On Oct. 24, 2018, 4:54 a.m., Apoorv Naik wrote: > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java > > Lines 259 (patched) > > <https://reviews.apache.org/r/69133/diff/1/?file=2102096#file2102096line259> > > > > Same multi-responsibility concept is being applied here. > > > > I feel it's better to break this down into checkState and fixState > > methods.
Please see response in other comment for line #866. > On Oct. 24, 2018, 4:54 a.m., Apoorv Naik wrote: > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java > > Lines 866 (patched) > > <https://reviews.apache.org/r/69133/diff/1/?file=2102096#file2102096line866> > > > > This method is performing two different actions. Ideally a method > > should only perform one action. > > > > Also when I look at checkState method signature, the initial impression > > I get is that this would perform a diagnostic kind of operation, but > > internally it does some corrections as well on the basis of the boolean > > flag. If possible, please make separate methods to address these actions. Making separate methods will cause duplicate codes. I thought of naming the method as checkAndFixState(); but that would also be confusing as 'fix' is to be done only if parameter asks to; perhaps checkAndFixIfNeeded().. that would be confusing too. Decided to leave it checkState() (ref - 'fsck' command, File System Consistency checK, does repair as well optionally). - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69133/#review209947 ----------------------------------------------------------- On Oct. 24, 2018, 8:20 a.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69133/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2018, 8:20 a.m.) > > > Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval > bhatt, Kapildeo Nayak, Nixon Rodrigues, and Sarath Subramanian. > > > Bugs: ATLAS-2934 > https://issues.apache.org/jira/browse/ATLAS-2934 > > > Repository: atlas > > > Description > ------- > > This patch adds a REST API to scan specified entities for inconsistency of > __traitNames property value and fix it. If the property value does not match > the classification vertices associated with the entity, the value will be > updated to reflect the associated vertices. > > > Diffs > ----- > > > intg/src/main/java/org/apache/atlas/model/instance/AtlasCheckStateRequest.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/instance/AtlasCheckStateResult.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java > 6a1a88f4f > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java > 449a9588e > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java > 0445e2798 > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportSkipLineageTest.java > c4682b8bc > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java > 089ff08d1 > webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java > b53ac69c4 > webapp/src/test/java/org/apache/atlas/web/resources/AdminResourceTest.java > 2f4b3d9bb > > > Diff: https://reviews.apache.org/r/69133/diff/2/ > > > Testing > ------- > > Verified that the new API detects and fixes __traitNames property, for the > inconsistencies introduced. Here is a sample response from the call: > > { > "state": "FIXED", > "entitiesScanned": 19021, > "entitiesFixed": 1, > "entitiesNotFixed": 0, > "entitiesOk": 19020, > "entitiesPartiallyFixed": 0, > "entities": { > "e87a1236-eeb4-4474-8cfd-a82117b8d1cd": { > "guid": "e87a1236-eeb4-4474-8cfd-a82117b8d1cd", > "name": "testDb.testTable@myCluster", > "state": "FIXED", > "status": "ACTIVE", > "typeName": "hive_table", > "issues": [ > "incorrect property: __traitNames has missing > classifications: [DATA_QUALITY]", > "incorrect property: __traitNames has unassigned > classifications: [UNKNOWN_TAG2, UNKNOWN_TAG]" > ] > } > } > } > > > Thanks, > > Madhan Neethiraj > >
