----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71482/#review218202 -----------------------------------------------------------
common/src/main/java/org/apache/atlas/repository/Constants.java Lines 206 (patched) <https://reviews.apache.org/r/71482/#comment305769> SupportFileFormat => GlosasaryImportSupportedFileFormats intg/src/main/java/org/apache/atlas/ApplicationProperties.java Lines 63 (patched) <https://reviews.apache.org/r/71482/#comment305770> - TEMP_FILE_LOCATION - if this is specific to glossary import, consider renaming to GLOSSARY_IMPORT_TEMP_DIRECTORY - also, consider moving this to AtlasConfiguration, similar to IMPORT_TEMP_DIRECTORY intg/src/main/java/org/apache/atlas/AtlasErrorCode.java Lines 162 (patched) <https://reviews.apache.org/r/71482/#comment305764> ATLAS-400-00-881 => ATLAS-400-00-9B repository/pom.xml Lines 230 (patched) <https://reviews.apache.org/r/71482/#comment305765> define the version in parent pom.xml, and reference here with the property name - like: ${opencsv.version} Please review #236, #241 as well for similar update. repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java Lines 542 (patched) <https://reviews.apache.org/r/71482/#comment305781> Consider rewritting: if(!(record.length>0) || !StringUtils.isNotBlank(record[0])){ as: if (record.length < 1 || StringUtils.isBlank(record[0])) { repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java Lines 547 (patched) <https://reviews.apache.org/r/71482/#comment305785> 'glossaryName' is likely to be referenced in multiple rows in the imported file. I suggest to cache the retrieved glossaryGuid (like #552) and avoid the call at #549. repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java Lines 549 (patched) <https://reviews.apache.org/r/71482/#comment305782> Replace "invalidNameChars[1]" with ".", or a constant like: static final String SEP_TYPENAME_ATTR = "."; review for other such use of invalidNameChars. repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java Lines 572 (patched) <https://reviews.apache.org/r/71482/#comment305771> getGlossaryTermHeaders() => getGlossaryImportHeaderRow() repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java Lines 581 (patched) <https://reviews.apache.org/r/71482/#comment305774> termName => attrName repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java Lines 585 (patched) <https://reviews.apache.org/r/71482/#comment305772> Is it necessary to include relationship names in 'header row'? If yes, can you please give few example relationship names that are needed in import. repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java Lines 595 (patched) <https://reviews.apache.org/r/71482/#comment305773> Consider using StringUtils.join(), instead of stripping first and last char from toString(). repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java Lines 598 (patched) <https://reviews.apache.org/r/71482/#comment305786> populateGlossaryTermObject() seems to expect the columns in the following order. This order may not be the same in 'glossaryTermHeaderListAsString', which is created by iterating through atlasEntityDef.getAttributeDefs() and atlasEntityDef.getRelationshipAttributeDefs(). Please review and update; it will be helpful to return 'headerRow' hardcoded with the same order as expected in populateGlossaryTermObject(). - glossaryName - termName - shortDescription - longDescription - examples - abbreviation - usage - additionalAttributes - tranlationTerms - validValuesFor - synonyms - replacedBy - validValues - replacementTerms - isA - anchor - classifies - preferredToTerms - perferredTerms repository/src/main/java/org/apache/atlas/util/FileUtils.java Lines 59 (patched) <https://reviews.apache.org/r/71482/#comment305767> - which directory would this file (named "template") be created in? - every call to getGlossaryTermTemplate() will write the given data to the same file. Does it overwrite or append the contents? - When does the "template" file get deleted? repository/src/main/java/org/apache/atlas/util/FileUtils.java Lines 69 (patched) <https://reviews.apache.org/r/71482/#comment305768> Consider using FilenameUtils.getExtension(fileName) - http://commons.apache.org/proper/commons-io/javadocs/api-2.5/org/apache/commons/io/FilenameUtils.html#getExtension(java.lang.String) webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java Lines 957 (patched) <https://reviews.apache.org/r/71482/#comment305776> "/template" => "/importHeaderRow" webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java Lines 959 (patched) <https://reviews.apache.org/r/71482/#comment305777> Should the return type here be a 'File'? It will be simpler to return a String. Please review and update. webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java Lines 976 (patched) <https://reviews.apache.org/r/71482/#comment305766> "/importGlossaryData" => "/import" - Madhan Neethiraj On Oct. 14, 2019, 1:21 p.m., mayank jain wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71482/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2019, 1:21 p.m.) > > > Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, > and Sarath Subramanian. > > > Bugs: ATLAS-3423 > https://issues.apache.org/jira/browse/ATLAS-3423 > > > Repository: atlas > > > Description > ------- > > This patch consists implementation for 2 end points first for template > download and other for csv file upload with term details also the Unit Test > cases for both the end points. > > * The 1st endpoint {glossary/template} return template file this would be > type of format of data that shows how the data needs to be populated by user > in the file. > > http://localhost:21000/api/atlas/v2/glossary/template > > Template structure:- > > GlossaryName, TermName, ShortDescription, LongDescription, Examples, > Abbreviation, Usage, AdditionalAttributes, TranslationTerms, ValidValuesFor, > Synonyms, ReplacedBy, ValidValues, ReplacementTerms, SeeAlso, > TranslatedTerms, IsA, Antonyms, Classifies, PreferredToTerms, PreferredTerms > Fruits,Apple5,SD4,LD4,"EXAMPLE","ABBREVIATION","USAGE",,"Footwear:B4","Footwear:B4","Footwear:B4","Footwear:B4","Footwear:B4","Footwear:B4","Footwear:B4","Footwear:B4","Footwear:B4","Footwear:B4","Footwear:B4","Footwear:B4","Footwear:B4" > > > * The 2nd endpoint {glossary/importGlossaryData} (file upload) would actually > parse the Data into AtlasObjects and further create the AtlasGlossaryTerms > inside Glossary. > > curl -v -g POST -u admin:admin -H "Content-Type: multipart/form-data" -H > "Cache-Control: no-cache" -F file=@template_6.csv > "http://localhost:21000/api/atlas/v2/glossary/importGlossaryData"; > > > Note:- > > While populating the data in the csv file each record should be maintained > in single Line (enter command within the record would result in parsing the > second line as a new record). > > The downloaded template needs to be saved as whateverTheFileNameIs.csv > explicitly. > > If the file is been succefully uploaded then the AtlasGlossaryTerm would be > returned or else List of Errors would returned for user to rectify them > further. > > > Diffs > ----- > > common/src/main/java/org/apache/atlas/repository/Constants.java 357affd > intg/src/main/java/org/apache/atlas/ApplicationProperties.java d3afd53 > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 9812356 > > intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasGlossaryHeader.java > 660514b > repository/pom.xml 802d587 > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java > 9229d2d > repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java > cdc3f07 > repository/src/main/java/org/apache/atlas/glossary/GlossaryUtils.java > 9625f94 > repository/src/main/java/org/apache/atlas/util/FileUtils.java PRE-CREATION > repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java > be1698f > repository/src/test/resources/csvFiles/empty.csv PRE-CREATION > repository/src/test/resources/csvFiles/incorrectFile.csv PRE-CREATION > repository/src/test/resources/csvFiles/template_1.csv PRE-CREATION > repository/src/test/resources/excelFiles/template_1.xlsx PRE-CREATION > webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java 151aa6b > > > Diff: https://reviews.apache.org/r/71482/diff/7/ > > > Testing > ------- > > Tested both the endpoint with curl call. > > Tested upload term csv with around 100 records from curl. > > > Thanks, > > mayank jain > >
