[GitHub] metron issue #1010: METRON-1548: Remove hardcoded source:type from Alerts UI
Github user merrimanr commented on the issue: https://github.com/apache/metron/pull/1010 +1 pending travis ---
[GitHub] metron issue #1010: METRON-1548: Remove hardcoded source:type from Alerts UI
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/1010 I think the general ES migration to source.type is outside the scope of this PR. That would have to be part of a larger migration anyway since you'd have to reindex data on an existing metron cluster to do that. I'm sure we'll want to think about it at some point, though. ---
[GitHub] metron issue #1010: METRON-1548: Remove hardcoded source:type from Alerts UI
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/1010 @merrimanr After thinking about this a bit, is that actually a serious problem? This setting is likely to only be done on initial install (e.g. when selecting Solr or ES). Switching midlife is unlikely to be a problem for the vast majority of users. Most users should never see the problem of it being switched because they'd never see `source:type` instead of `source.type` or vice versa, because you'd have to reindex all your old data into the new format anyway. We'd need to improve documentation in the Solr feature branch to ensure that users initially set it up appropriately. Another way of going about this might be to have the app itself get config updates on the fly similar to how our topologies do, given that this particular one is from ZK. At that point, you could do the mapping from old source type to new source type and update the storage appropriately. I'm sure there's way more to it than just doing that though (when do you update UI for users, what are the potential snags in updating that when users are querying, etc.). ---
[GitHub] metron issue #1010: METRON-1548: Remove hardcoded source:type from Alerts UI
Github user merrimanr commented on the issue: https://github.com/apache/metron/pull/1010 Most of this worked, I could only find one issue. We store a user's selected columns in local storage right now. These columns are selected in the Configure Table view and the list of available columns comes from a REST call that returns ES (or Solr) metadata. The available column list will always be correct since that information is coming straight from the index templates or schemas but the column name in local storage will become incorrect since it's still the old source type. There is an attempt to just transform the name in the selected column list but I think this is flawed (I added an inline comment on that). I am not sure what the best way to handle this is. I feel like if we find ourselves hardcoding either "source:type" or "source.type" we're doing something wrong. One idea is to limit the source.type.field property to be either "source:type" or "source.type". This would at least allow us to perform those hardcoded checks to see if we need to substitute the new value. Otherwise we'll need to evolve our local storage model by using boolean fields like "includeSourceType" instead of the actual source.type.field value. This would make upgrading harder though. ---
[GitHub] metron issue #1010: METRON-1548: Remove hardcoded source:type from Alerts UI
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/1010 After the fix for the table header bug, I wasn't able to repeat it. Also, my PR was merged into here to have for the REST API to pull the "source.type.field" from the global config to ensure numbers 2 and 3 are taken care of. I'm going to refrain from voting on this PR given that contribution. ---
[GitHub] metron issue #1010: METRON-1548: Remove hardcoded source:type from Alerts UI
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/1010 I spun this up, changed the config, restarted the UI and had a couple comments. 1. Every time you log in, it keeps adding new source.type fields in the table. Unselecting these in the options doesn't work properly (only the top one toggles no matter which I click on). Possibly doubles every time? 2. The filters on the left still say "source:type", even after the config is changed. 3. The group by still says "source:type", even after the config is changed. This grouping still works fine (which I would expect if it's still grouping on source:type). I would expect both the filter and group by to act on the configured `source.type.field` by default. Otherwise, the user has to know what's going on and change it manually. I'd also like to add a bit in the documentation that updating the `source.type.field` requires a UI restart. This is pretty typical of the UI, but most global config things get picked up automagically (e.g. topology changes and such), so a quick blurb would be nice. ---
[GitHub] metron issue #1010: METRON-1548: Remove hardcoded source:type from Alerts UI
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/1010 Also, UI for the subsytem should be fine. It's purely informational. If you wanted to be more explicit, you could put "Alerts UI" or something similar, but I think just "UI" is fine. ---
[GitHub] metron issue #1010: METRON-1548: Remove hardcoded source:type from Alerts UI
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/1010 @sardell Looks like Travis failed for unrelated reasons (which I'm curious about on it's own). Could you just close and reopen the PR to kick Travis off again? ---
[GitHub] metron issue #1010: METRON-1548: Remove hardcoded source:type from Alerts UI
Github user sardell commented on the issue: https://github.com/apache/metron/pull/1010 @justinleet I've changed the property to be `source.type.field` and added the property to the documentation at https://github.com/apache/metron/tree/master/metron-platform/metron-common#global-configuration. I was slightly confused about the subset field, but I took an educated guess and entered `UI` as the value. Let me know if this is not correct and I'll update it. ---
[GitHub] metron issue #1010: METRON-1548: Remove hardcoded source:type from Alerts UI
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/1010 Thanks for the contribution, getting this cleaned up is really helpful! Could you update https://github.com/apache/metron/tree/master/metron-platform/metron-common#global-configuration with the new config property? I'd also like to see the property be something like `source.type.field` to be a bit more explanatory and consistent with the other non-Object type fields. ---
