metron-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nickwallen <...@git.apache.org>
Subject [GitHub] metron pull request #1254: METRON-1849 Elasticsearch Index Write Functionali...
Date Wed, 05 Dec 2018 13:36:28 GMT
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`?



---

Mime
View raw message