Re: Review Request 72566: ATLAS-1798 : Fix Findbugs problems in repository module

2020-06-16 Thread Sarath Subramanian 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72566/#review221018
---




repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java
Line 265 (original)


why is this logic removed? please revert if not intended.

how will sortBy and sortOrder be normalized?



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java
Line 629 (original)


why reverse this logic?

if isUnique is null or false - it should be LIST.

Revert if not intended.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
Line 2249 (original), 2235 (patched)


these checks are confusing, revert to old one.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
Line 2273 (original), 2260 (patched)


these checks are confusing, revert to old one.



repository/src/main/java/org/apache/atlas/util/FileUtils.java
Lines 36 (patched)


nit: unused import. consider removing it.


- Sarath Subramanian


On June 10, 2020, 7:56 p.m., mayank jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72566/
> ---
> 
> (Updated June 10, 2020, 7:56 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
> -
> 
>   build-tools/src/main/resources/findbugs-exclude.xml da6c58d 
>   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 
>

Re: Review Request 72566: ATLAS-1798 : Fix Findbugs problems in repository module

2020-06-10 Thread mayank jain 


> On June 10, 2020, 4:38 a.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java
> > Line 1081 (original), 1080 (patched)
> > 
> >
> > What was the findbug issue reported here? Can you please review if 
> > String.format() handles '{}' as placeholder for arguments?

[INFO] Format-string method String.format(String, Object[]) called with format 
string "Received unknown property{} for attribute {}'s " wants 0 arguments but 
is given 2 in 
org.apache.atlas.repository.store.bootstrap.AtlasTypeDefStoreInitializer

Using String.format("Received unknown property{} for attribute {}'s ", 
entry.getKey(), atlasAttributeDef.getName()); leads  to above error.


> On June 10, 2020, 4:38 a.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java
> > Line 627 (original), 623 (patched)
> > 
> >
> > isUnique can be null here, why is not neceessary to handle this case 
> > here?

This was leading to following findbugs error

Suspicious comparison of Boolean references in 
org.apache.atlas.repository.store.graph.v2.AtlasStructDefStoreV2.toAttributeDefFromJson(AtlasStructDef,
 Map, AtlasTypeDefGraphStoreV2) 
[org.apache.atlas.repository.store.graph.v2.AtlasStructDefStoreV2] At 
AtlasStructDefStoreV2.java:[line 623] RC_REF_COMPARISON_BAD_PRACTICE_BOOLEAN

Though i have reverted it back as before as it might lead to some error.


> On June 10, 2020, 4:38 a.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/pc/EntityConsumer.java
> > Line 121 (original)
> > 
> >
> > 'result' is not used; however, wouldn't removing call to 
> > entityStoreBulk.createOrUpdateForImportNoCommit() cause no import to be 
> > performed??

i have fixed this.


- mayank


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72566/#review220984
---


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
> -
> 
>   build-tools/src/main/resources/findbugs-exclude.xml da6c58d 
>   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
>

Re: Review Request 72566: ATLAS-1798 : Fix Findbugs problems in repository module

2020-06-10 Thread mayank jain 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72566/
---

(Updated June 11, 2020, 2:56 a.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 (updated)
-

  build-tools/src/main/resources/findbugs-exclude.xml da6c58d 
  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

Re: Review Request 72566: ATLAS-1798 : Fix Findbugs problems in repository module

2020-06-09 Thread Madhan Neethiraj 

---
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)


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)


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)


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)


- 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)


- 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)


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)


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)


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)


'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 
>   
>

Re: Review Request 72566: ATLAS-1798 : Fix Findbugs problems in repository module

2020-06-09 Thread mayank jain 


> On June 8, 2020, 2:39 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java
> > Line 265 (original)
> > 
> >
> > Why is this code block removed? If necessary, please rebase.

This code block was showing Dead Code by findbugs, initially line #276 was 
showing dead code so i removed and then the code above started showing dead 
code. That is why i had to remove the whole code block.


> On June 8, 2020, 2:39 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java
> > Line 905 (original), 905 (patched)
> > 
> >
> > This update doesn't look right. Why is 'ret' initialized in #905, and 
> > have it reassinged in #915. What this the issue flagged for the existing 
> > code?

This code block was initially showing "Useless object stored in variable ret of 
method" I tried to make less changes to the existing code as possible  though i 
have made better changes now, please have a look.


> On June 8, 2020, 2:39 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLLexer.java
> > Line 86 (original), 88 (patched)
> > 
> >
> > Use of Arrays.copyOf() could cause excessive memory allocation. Why is 
> > this necessary - in AtlasDSLLexer.java, AtlasDSLParser.java?

By statement "return tokenNames;" findBugs was throwing 
"org.apache.atlas.query.antlr4.AtlasDSLLexer.getTokenNames() may expose 
internal representation by returning AtlasDSLLexer.tokenNames 
[org.apache.atlas.query.antlr4.AtlasDSLLexer] At AtlasDSLLexer.java:[line 88] 
EI_EXPOSE_REP"
That is why i had to opt for this solution.


- mayank


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72566/#review220966
---


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 
>   
>

Re: Review Request 72566: ATLAS-1798 : Fix Findbugs problems in repository module

2020-06-09 Thread mayank jain 

---
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 (updated)
-

  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

Re: Review Request 72566: ATLAS-1798 : Fix Findbugs problems in repository module

2020-06-08 Thread Madhan Neethiraj 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72566/#review220966
---




repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Line 147 (original), 147 (patched)


Consider replacing #147, #148 with:
  for (Object entryObj : map.entrySet()) {
Map.Entry entry = (Map.Entry) entryObj;
Objectkey   = entry.getKey();
Objectvalue = entry.getValue();

Also, apply the same for other such changes as well.



repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java
Line 265 (original)


Why is this code block removed? If necessary, please rebase.



repository/src/main/java/org/apache/atlas/discovery/SearchAggregatorImpl.java
Line 96 (original), 96 (patched)


aggregationMetricName => entry



repository/src/main/java/org/apache/atlas/discovery/SearchAggregatorImpl.java
Line 98 (original), 98 (patched)


Consider replacing "aggregationMetricName.getKey()" with:
  String aggregationMetricName = entry.getKey();

and use aggregationMetricName in #98, #99, 105.



repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java
Line 905 (original), 905 (patched)


This update doesn't look right. Why is 'ret' initialized in #905, and have 
it reassinged in #915. What this the issue flagged for the existing code?



repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLLexer.java
Line 86 (original), 88 (patched)


Use of Arrays.copyOf() could cause excessive memory allocation. Why is this 
necessary - in AtlasDSLLexer.java, AtlasDSLParser.java?



repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
Line 211 (original), 211 (patched)


Remove #211, instead of commenting.



repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java
Line 346 (original), 346 (patched)


Instead of commenting unused code, please remove.



repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java
Line 357 (original), 357 (patched)


If isSkipConnectedFetch is unused, please remove all references to this, 
instead of commenting it out.


- Madhan Neethiraj


On June 4, 2020, 2:33 a.m., mayank jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72566/
> ---
> 
> (Updated June 4, 2020, 2:33 a.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 
>

Re: Review Request 72566: ATLAS-1798 : Fix Findbugs problems in repository module

2020-06-08 Thread Jayendra Parab 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72566/#review220965
---




repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java
Line 309 (original), 312 (patched)


Remove commented code, its there in multiple places, remove from there as 
well



repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java
Line 537 (original), 537 (patched)


Remove this declaration and move it to line 572


- Jayendra Parab


On June 4, 2020, 2:33 a.m., mayank jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72566/
> ---
> 
> (Updated June 4, 2020, 2:33 a.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 
>   
>

Review Request 72566: ATLAS-1798 : Fix Findbugs problems in repository module

2020-06-03 Thread mayank jain 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72566/
---

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/EntityGraphMapper.java
 2ed524f 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
 757fcb1