----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72566/#review220984 -----------------------------------------------------------
repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLLexer.java Line 105 (original), 107 (patched) <https://reviews.apache.org/r/72566/#comment309737> I strongly suggest to avoid Arrays.copyOf(), since it can cause excessive memory usage. Consider ignoring EI_EXPOSE_REP with an entry in build-tools/src/main/resources/findbugs-exclude.xml. repository/src/main/java/org/apache/atlas/repository/impexp/ZipSource.java Lines 150 (patched) <https://reviews.apache.org/r/72566/#comment309738> Instead of hardcoding string "UTF-8", consider using StandardCharsets.UTF_8.name() repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java Lines 282 (patched) <https://reviews.apache.org/r/72566/#comment309739> Instead of hardcoding string "UTF-8", consider using StandardCharsets.UTF_8.name() repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationService.java Lines 132 (patched) <https://reviews.apache.org/r/72566/#comment309740> - verify that 'fs' is not null, before calling fs.close() - also, consider avodinig a new 'try' (#128) by adding 'finally' for the 'try' at #122 repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java Lines 139 (patched) <https://reviews.apache.org/r/72566/#comment309741> - verify that 'fs' is not null, before calling fs.close() - also, consider avodinig a new 'try' (#130) by adding 'finally' for the 'try' at #127 repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java Line 1081 (original), 1080 (patched) <https://reviews.apache.org/r/72566/#comment309742> What was the findbug issue reported here? Can you please review if String.format() handles '{}' as placeholder for arguments? repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java Lines 308 (patched) <https://reviews.apache.org/r/72566/#comment309743> To be consistent with rest of Atlas coding style, use spaces to seprate keywords and operations, like: if (ret != null) { repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java Line 627 (original), 623 (patched) <https://reviews.apache.org/r/72566/#comment309744> isUnique can be null here, why is not neceessary to handle this case here? repository/src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/pc/EntityConsumer.java Line 121 (original) <https://reviews.apache.org/r/72566/#comment309745> 'result' is not used; however, wouldn't removing call to entityStoreBulk.createOrUpdateForImportNoCommit() cause no import to be performed?? - Madhan Neethiraj On June 9, 2020, 12:44 p.m., mayank jain wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72566/ > ----------------------------------------------------------- > > (Updated June 9, 2020, 12:44 p.m.) > > > Review request for atlas, Jayendra Parab, Madhan Neethiraj, Nixon Rodrigues, > and Sarath Subramanian. > > > Bugs: ATLAS-1798 > https://issues.apache.org/jira/browse/ATLAS-1798 > > > Repository: atlas > > > Description > ------- > > Currently Findbugs complaints about some problems (see attachment) in the > repository module. They should be fixed to get the code more reliable. > > > Diffs > ----- > > repository/src/main/java/org/apache/atlas/GraphTransactionInterceptor.java > 57e454a > > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java > dd4d1b4 > > repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java > 56956e6 > > repository/src/main/java/org/apache/atlas/discovery/SearchAggregatorImpl.java > e8f7dbc > repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java > 804c694 > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java > d630f66 > repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java > 2c84ec7 > repository/src/main/java/org/apache/atlas/glossary/GlossaryUtils.java > 2a2cebb > repository/src/main/java/org/apache/atlas/query/AtlasDSL.java b8a744b > repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java > 801e898 > repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLLexer.java > 142b9ca > repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLParser.java > 6b33edb > > repository/src/main/java/org/apache/atlas/repository/audit/AbstractStorageBasedAuditRepository.java > 1aac375 > > repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java > 69d373d > > repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java > 79527ac > > repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java > 9fca744 > > repository/src/main/java/org/apache/atlas/repository/converters/AtlasArrayFormatConverter.java > c335f0a > > repository/src/main/java/org/apache/atlas/repository/converters/AtlasInstanceConverter.java > 6fc0c65 > > repository/src/main/java/org/apache/atlas/repository/converters/AtlasMapFormatConverter.java > 0eacd8e > > repository/src/main/java/org/apache/atlas/repository/converters/AtlasStructFormatConverter.java > ae92b8b > > repository/src/main/java/org/apache/atlas/repository/graph/FullTextMapperV2.java > 497a877 > > repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java > 4a09b08 > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java > 7b7ec65 > > repository/src/main/java/org/apache/atlas/repository/impexp/AtlasServerService.java > 542106f > > repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java > 0491a85 > > repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java > 1d29bf8 > > repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransforms.java > a2f592c > repository/src/main/java/org/apache/atlas/repository/impexp/ZipSink.java > 6375454 > repository/src/main/java/org/apache/atlas/repository/impexp/ZipSource.java > 812add9 > > repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceDirect.java > 04342fa > > repository/src/main/java/org/apache/atlas/repository/impexp/ZipSourceWithBackingDirectory.java > 7963800 > > repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationService.java > 0a2257e > > repository/src/main/java/org/apache/atlas/repository/migration/ZipFileMigrationImporter.java > d56261f > > repository/src/main/java/org/apache/atlas/repository/ogm/AtlasServerDTO.java > 2f7ca11 > > repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java > 8e7c1b3 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java > 3f8503a > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java > 9ffede4 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityChangeNotifier.java > 0dc3193 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityDefStoreV2.java > e5153de > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java > 89076c1 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java > 8d74489 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java > 9a45f00 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasTypeDefGraphStoreV2.java > ed17b92 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/BulkImporterImpl.java > 8e17fd4 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java > 2ed524f > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java > 757fcb1 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/UniqAttrBasedEntityResolver.java > d1c3bde > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/pc/EntityConsumer.java > b73988f > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/pc/EntityCreationManager.java > 734add6 > repository/src/main/java/org/apache/atlas/util/AtlasMetricsCounter.java > 10319d0 > repository/src/main/java/org/apache/atlas/util/AtlasMetricsUtil.java > beb90e6 > repository/src/main/java/org/apache/atlas/util/FileUtils.java 66ade26 > > repository/src/test/java/org/apache/atlas/discovery/BasicSearchClassificationTest.java > 9b16e91 > repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java 5ace379 > > repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java > 959aa11 > repository/src/test/java/org/apache/atlas/repository/AtlasTestBase.java > c2668b1 > > repository/src/test/java/org/apache/atlas/repository/audit/AtlasAuditServiceTest.java > 54f75cc > > repository/src/test/java/org/apache/atlas/repository/audit/AuditRepositoryTestBase.java > bf4f395 > > repository/src/test/java/org/apache/atlas/repository/audit/CassandraAuditRepositoryTest.java > 26d3a60 > > repository/src/test/java/org/apache/atlas/repository/audit/InMemoryAuditRepositoryTest.java > 3bdfcf9 > > repository/src/test/java/org/apache/atlas/repository/impexp/AtlasServerServiceTest.java > 91ffc27 > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportIncrementalTest.java > 0e3955d > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportServiceTest.java > 8e19dc4 > > repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsShaperTest.java > c2b9dbb > > repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsTest.java > 1959576 > > repository/src/test/java/org/apache/atlas/repository/impexp/IncrementalExportEntityProviderTest.java > ed6c12d > > repository/src/test/java/org/apache/atlas/repository/impexp/RelationshipAttributesExtractorTest.java > 5f41cc9 > > repository/src/test/java/org/apache/atlas/repository/impexp/ReplicationEntityAttributeTest.java > ebdc7b5 > > repository/src/test/java/org/apache/atlas/repository/impexp/UniqueListTest.java > 2118df9 > > repository/src/test/java/org/apache/atlas/repository/impexp/ZipDirectTest.java > faa31c3 > > repository/src/test/java/org/apache/atlas/repository/impexp/ZipSinkTest.java > cf6d16b > > repository/src/test/java/org/apache/atlas/repository/migration/MigrationProgressServiceTest.java > 33125c8 > > repository/src/test/java/org/apache/atlas/repository/migration/TypesDefScrubberTest.java > d40ca82 > > repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java > b654638 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2Test.java > eaffac1 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java > b9cbef1 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityTestBase.java > 752f1ac > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2Test.java > 24683a5 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/BulkImportPercentTest.java > 1ae98ce > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/ClassificationAssociatorTest.java > d31b464 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/InverseReferenceUpdateV2Test.java > 6364fd4 > > repository/src/test/java/org/apache/atlas/repository/tagpropagation/ClassificationPropagationTest.java > 80e20be > > repository/src/test/java/org/apache/atlas/repository/userprofile/UserProfileServiceTest.java > eeab3bc > > repository/src/test/java/org/apache/atlas/utils/ObjectUpdateSynchronizerTest.java > 03ebae4 > repository/src/test/java/org/apache/atlas/utils/TestLoadModelUtils.java > f175386 > > > Diff: https://reviews.apache.org/r/72566/diff/2/ > > > Testing > ------- > > Tried running all the Test Cases and they were running fine. > > > Thanks, > > mayank jain > >
