nickwallen commented on issue #1454: METRON-2172 Solr Updates Not Tested in Integration Test URL: https://github.com/apache/metron/pull/1454#issuecomment-511993340 I found a problem with this change. The Alerts UI does not properly display comments when indexing into Elasticsearch. See screenshot.  The problem is that we serialize comments into Elasticsearch differently than we do Solr. For Elasticsearch we write the comments as a list of maps. For Solr, we write a list of JSON strings. I do not understand why we serialize these differently, but I definitely don't want to change that behavior in this PR. With this change, it wrote the comments as a list of JSON strings for both Solr and Elasticsearch. When running against Elasticsearch, the Alerts UI is then passed a raw JSON string that it does not know how to handle; hence the"Invalid Date" error in the Alerts UI. To really fix this, I would need to refactor deeper into the Elasticsearch side of the DAOs and have them also use the `DocumentBuilder` abstraction. I would also need to add a test to catch this issue. As mentioned in the PR description we (de)serialize comments in many different places and this should ultimately be cleaned up and more thoroughly tested. I am going to shelve this refactor for later. Until then, the minimal necessary fixes to test current behavior is in #1456 . Its less than ideal, but it gets the job done.
---------------------------------------------------------------- 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
