nickwallen commented on a change in pull request #1403: METRON-2109: Add option to use Metron GUID as the id in Elasticsearch URL: https://github.com/apache/metron/pull/1403#discussion_r282888202
########## File path: metron-platform/metron-elasticsearch/src/test/java/org/apache/metron/elasticsearch/writer/ElasticsearchWriterTest.java ########## @@ -243,6 +257,54 @@ public void shouldWriteSuccessfullyWhenMissingGUID() { assertTrue(response.getSuccesses().contains(new MessageId("message1"))); } + @Test + public void shouldWriteManySuccessfullyWithMetronId() { Review comment: I think all of the tests you added cover the write side. We really need to test this change on the read-side. * All the existing read-side tests are run with an auto-generated ID. Ideally, we could somehow run them all once using auto-gen ID and again using Metron GUID. This may not be feasible though. * If not, we need to add/edit some read-side tests to ensure that we can read/update indices written either with the Metron GUID or the auto-generated ID. * We should also test this functionality when there are mixed indices, either across different sensors or even within the same sensor. For example... * Bro is configured to use auto-gen ID, but Snort is using Metron GUID. * The Bro index was using auto-gen ID for the past 2 months, but then it was switched to GUID. Searches will span both ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services