Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/699
I have tested this in full dev and it works as advertised. I haven't
noticed any obvious bugs and it was fairly easy to install and start. I would
suggest we add a note in the "Development Setup" section of the metron-alerts
README that tells users how to access the UI. I know it's "localhost:4200" and
to login as "user/password" because I'm familiar with the management UI but
others probably won't know that.
I do not think the DataSource/ElasticSearchLocalstorageImpl/RestApiImpl
abstraction is correct. We only have a single Metron REST implementation so I
don't see why we would need to abstract this. I know things are in a weird
state switching from directly calling ES to going through the REST layer, but I
don't think this moves us in the right direction. I would rather see a
standalone SearchService class that matches the SearchController in the REST
layer. As we create more REST controllers and endpoints, the services on the
client-side would also be created/updated to match. Eventually the
DataSource/ElasticSearchLocalstorageImpl classes would be empty and at that
point we would know we're done moving the calls to REST. With this approach,
all the functions will end up in RestApiImpl and that's not the end state we
want. Sure we could reorganize them after all the DataSource methods are
implemented in RestApiImpl but why not do it incrementally?
@nickwallen has already put in time testing and this is more of a
style/organization issue so I would be ok with deferring this to a future PR.
However any +1 from me in the future would require we work this out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
