[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-12-12 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 I'm good +1 ---

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-12-12 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/834 As far as THIS PR, I think it has the 2 +1s that I was waiting for. As for a long-range fix, I think it's going to happen outside of this PR. Did I miss anything? ---

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-12-12 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 I expect to address this in the "777" feature branch/ parser effort. ---

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-12-12 Thread merrimanr
Github user merrimanr commented on the issue: https://github.com/apache/metron/pull/834 After reading this whole thread I agree with @ottobackwards. If we installed and started all services and THEN installed our parsers separately this whole issue goes away. If a parser's template

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-12-12 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 Where are we at with this? ---

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 My comment is more 'big picture'. I don't think it nec. applies to how this PR is resolved. ---

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread mmiklavc
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/834 @ottobackwards I need to think through the use cases and implications more (e.g. what about a non-ES situation?), but I think your points are valid. ---

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 my comment was re: "It appears to me that we cannot add new templates via the current Ambari mechanism without modifying a release/MPack" ---

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 This is why I put the templates with the parsers in 777. Although I did not have the templates installing from there because list discussion was tough, mainly because talking about this in

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread mmiklavc
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/834 @nickwallen and @cestella you both make some good points here. The upshot is that this part of our architecture is inextricably tied to ES and in reality we should have another abstraction here. It

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/834 I ran this up in full-dev and index start functions fine and data is written to the indices. ---

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/834 Nick's points are spot-on, I think. I think the core abstraction issue is that we're doing ES-specific things in the indexing section. We probably want to decouple indexing from ES a bit and have

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/834 Ok, so a couple of things here. I wanted to have a vigorous discussion, because this change has some pros and cons. At the moment, as Nick alluded, we do template install upon index

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread nickwallen
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/834 And for the record @anandsubbu did implement this as "fail fast" in his original PR. So I think he is all for this. I was the one who talked him out of failing fast here. I was wary of

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread nickwallen
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/834 I clearly see the advantages of failing fast here, but I do have a hesitation. I haven't even totally convinced myself of this. Maybe you guys can help me think down this line of thought and

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread mmiklavc
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/834 This is good. +1 by inspection. ---

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/834 Ah, yes, sorry, I should be more clear here. Starting the indexing topology should fail if the indexing templates are not installed. ---

[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...

2017-11-07 Thread nickwallen
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/834 Just a comment on the description of the JIRA, not the code change itself. Index templates can only be installed when starting the Indexing topology, not during install. During install,