[ 
https://issues.apache.org/jira/browse/METRON-1849?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16710074#comment-16710074
 ] 

ASF GitHub Bot commented on METRON-1849:
----------------------------------------

Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1254#discussion_r239065568
  
    --- Diff: 
metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchDao.java
 ---
    @@ -196,7 +196,7 @@ public ElasticsearchDao 
withRefreshPolicy(WriteRequest.RefreshPolicy refreshPoli
       }
     
       protected Optional<String> getIndexName(String guid, String sensorType) 
throws IOException {
    -    return updateDao.getIndexName(guid, sensorType);
    +    return updateDao.findIndexNameByGUID(guid, sensorType);
    --- End diff --
    
    > Also, would we want any parity between the updateDao's find method name 
vs the ElasticsearchDao's getIndexName method name?
    
    I found the [code here 
confusing](https://github.com/apache/metron/blob/89a2beda4f07911c8b3cd7dee8a2c3426838d161/metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchUpdateDao.java#L195-L197)
 and had me stuck on an issue for quite some time.  When both use 
`getIndexName` I have no idea what the logic is doing.  It tries one approach, 
then falls back to another, but since the methods are named the same, it 
doesn't tell me how they attempt to find the index name in a different way.
    
    With the rename, I feel it improves understanding in a glance [what this is 
doing 
now](https://github.com/apache/metron/blob/260ccc366b79ef53595dbfd097066040444b4eda/metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchUpdateDao.java#L179)
 and the differences between the primary approach versus the fallback.
    
    
    > Is sensorType not a component to retrieving the index name? 
    
    So you prefer the original function name?  Or you prefer `lookupIndexName`, 
`findIndexNameByGUIDAndSensor`?



> Elasticsearch Index Write Functionality Should be Shared
> --------------------------------------------------------
>
>                 Key: METRON-1849
>                 URL: https://issues.apache.org/jira/browse/METRON-1849
>             Project: Metron
>          Issue Type: Bug
>            Reporter: Nick Allen
>            Assignee: Nick Allen
>            Priority: Major
>
> The index write functionality is currently duplicated between the 
> ElasticsearchWriter and the ElasticsearchUpdateDao.  This functionality 
> should be de-duplicated and shared between the two.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to