----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72638/#review221212 -----------------------------------------------------------
client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 220 (patched) <https://reviews.apache.org/r/72638/#comment309992> Use of 'final' for method parameters are not of much help - especially for simple/short methods of this class. I suggest to remove 'final' from all method parameters - one less distraction in the code! client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Line 274 (original), 316 (patched) <https://reviews.apache.org/r/72638/#comment309991> getLineageInfo(): renaming existing method would break current callers. I suggest to retain the current method name. client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 630 (patched) <https://reviews.apache.org/r/72638/#comment309993> Consider organizing the class as below: - static fields - public fields - protected fields - private fields - constructors - public static methods - public methods - protected methods - private methods - protected static methods - private static methods It makes reading the code a little easier. client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 631 (patched) <https://reviews.apache.org/r/72638/#comment309995> Consider grouping methods in terms of functionality - like: type-defs, entity instances, search, admin, etc. It might be simper to group along the REST API the method calls. client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 634 (patched) <https://reviews.apache.org/r/72638/#comment309994> - deleteAtlasTypeByName() => deleteTypeByName() - consider avoiding 'atlas' in method and variable names - name => typeName client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 638 (patched) <https://reviews.apache.org/r/72638/#comment310002> guid => entityGuid - please review and update other such occurances client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 642 (patched) <https://reviews.apache.org/r/72638/#comment310003> type => typeName - please review and update other occurances client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 647 (patched) <https://reviews.apache.org/r/72638/#comment310004> name => attrName? client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 650 (patched) <https://reviews.apache.org/r/72638/#comment309996> Consider removing this commented line. client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 654 (patched) <https://reviews.apache.org/r/72638/#comment309997> getClassificationsByClassification() => getEntityClassifications() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 657 (patched) <https://reviews.apache.org/r/72638/#comment310001> addClassificationsByUniqueAttribute() => addEntityClassifications() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 661 (patched) <https://reviews.apache.org/r/72638/#comment310000> updateClassificationsByUniqueAttribute() => updateEntityClassifications() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 666 (patched) <https://reviews.apache.org/r/72638/#comment309998> deleteClassificationByUniqueAttribute() => removeEntityClassification() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 671 (patched) <https://reviews.apache.org/r/72638/#comment309999> deleteClassificationByGuid() => removeEntityClassification() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 698 (patched) <https://reviews.apache.org/r/72638/#comment310005> addOrUpdateBusinessAttributesByBName() => addOrUpdateBusinessAttributes() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 701 (patched) <https://reviews.apache.org/r/72638/#comment310006> removeBusinessAttributesByBName() => removeBusinessAttributes() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 711 (patched) <https://reviews.apache.org/r/72638/#comment310010> deleteLabels() => removeLabels() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 715 (patched) <https://reviews.apache.org/r/72638/#comment310007> - is fileDetail needed? - uploadedInputStream is not used? client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 719 (patched) <https://reviews.apache.org/r/72638/#comment310008> - setLabelsByTypeName() => setLabels() - I suggest to reorder the arguments: typeName, uniqAttributes, labels client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 723 (patched) <https://reviews.apache.org/r/72638/#comment310009> - addLabelsByTypeName() => addLabels() - I suggest to reorder the arguments: typeName, uniqAttributes, labels client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 727 (patched) <https://reviews.apache.org/r/72638/#comment310011> - deleteLabelsByTypeName() => removeLabels() - I suggest to reorder the arguments: typeName, uniqAttributes, labels client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 751 (patched) <https://reviews.apache.org/r/72638/#comment310012> if any of these parameters are optional, consider moving them towards end of the method; and add them to 'queryParams' only if they are not null. client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 780 (patched) <https://reviews.apache.org/r/72638/#comment310013> - What value should be provided to fieldName? Consider adding a comment. - if this value can be null, update #783 only if the value is not null client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 792 (patched) <https://reviews.apache.org/r/72638/#comment310014> - executeSavedSearchByName() => executeSavedSearch() - To be consistent with #774, please reorder the parameters: - userName, searchName client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 797 (patched) <https://reviews.apache.org/r/72638/#comment310015> Make sure to leave a blank like before start of a method. Please review and update other occurances as well. client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 798 (patched) <https://reviews.apache.org/r/72638/#comment310016> executeSavedSearchByGuid() => executeSavedSearch() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 801 (patched) <https://reviews.apache.org/r/72638/#comment310017> getQuickSearch() => quickSearch() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 811 (patched) <https://reviews.apache.org/r/72638/#comment310018> postQuickSearch() => quickSearch() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 828 (patched) <https://reviews.apache.org/r/72638/#comment310019> getDetailGlossary() => getGlossaryExtInfo() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 864 (patched) <https://reviews.apache.org/r/72638/#comment310020> partialUpdates => attributes client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 872 (patched) <https://reviews.apache.org/r/72638/#comment310021> partialUpdates => attributes client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 880 (patched) <https://reviews.apache.org/r/72638/#comment310022> partialUpdates => attributes client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 907 (patched) <https://reviews.apache.org/r/72638/#comment310023> getGlossaryTermHeadersByGuid() => getGlossaryTermHeaders() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 918 (patched) <https://reviews.apache.org/r/72638/#comment310024> getGlossaryCategoriesByGuid() => getGlossaryCategories() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 928 (patched) <https://reviews.apache.org/r/72638/#comment310025> getGlossaryCategoriesHeaders() => getGlossaryCategoryHeaders() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 979 (patched) <https://reviews.apache.org/r/72638/#comment310026> disassociateTermAssignmentFromEntities() is a duplicate of removeTermAssignmentFromEntities() - looking at GlossaryREST.java. I suggest to remove method removeTermAssignmentFromEntities() client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 992 (patched) <https://reviews.apache.org/r/72638/#comment310027> - produceTemplate() => getGlossaryTemplate() - consider returning a string client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 996 (patched) <https://reviews.apache.org/r/72638/#comment310028> - inputStream is not used? - what would callers send for fileDetail? - consider returning List<AtlasGlossaryTerm> - similar to GlossaryREST.importGlossaryData() - Madhan Neethiraj On July 10, 2020, 12:05 a.m., Jyoti Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72638/ > ----------------------------------------------------------- > > (Updated July 10, 2020, 12:05 a.m.) > > > Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath > Subramanian, and Sidharth Mishra. > > > Bugs: ATLAS-3875 > https://issues.apache.org/jira/browse/ATLAS-3875 > > > Repository: atlas > > > Description > ------- > > There are many missing API endpoints in AtlasClientV2. This solution is > adding functions corresponding to those missing APIs for the following Rest > endpoints. > > 1. TypeRest > 2. EntityRest > 3. LineageRest > 4. DiscoveryRest > 5. GlossaryRest > 6. RelationshipRest > > This will enable users to test and integrate with APIs more effectively via > AtlasCient. > > > Diffs > ----- > > client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java > 7b6d1d0f3 > client/common/src/main/java/org/apache/atlas/AtlasBaseClient.java e3d2ebc34 > webapp/pom.xml 3c55b4dd4 > webapp/src/test/java/org/apache/atlas/web/TestUtils.java e22a1c10d > webapp/src/test/java/org/apache/atlas/web/integration/BaseResourceIT.java > 680028892 > webapp/src/test/java/org/apache/atlas/web/integration/BasicSearchIT.java > 808f62354 > > webapp/src/test/java/org/apache/atlas/web/integration/EntityV2JerseyResourceIT.java > cc883d615 > > webapp/src/test/java/org/apache/atlas/web/integration/GlossaryClientV2IT.java > PRE-CREATION > > webapp/src/test/java/org/apache/atlas/web/integration/LineageClientV2IT.java > PRE-CREATION > > webapp/src/test/java/org/apache/atlas/web/integration/TypedefsJerseyResourceIT.java > 331ea2c47 > webapp/src/test/resources/json/search-parameters/attribute-filters.json > PRE-CREATION > > > Diff: https://reviews.apache.org/r/72638/diff/4/ > > > Testing > ------- > > IT has been added for newly created functions in AtlasClientV2. > Validated by running IT on local system. > > > Thanks, > > Jyoti Singh > >
