[GitHub] metron issue #1084: METRON-1644: Support parser chaining

2018-07-10 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/1084 1 nit, other than that +1 ---

[GitHub] metron issue #1084: METRON-1644: Support parser chaining

2018-07-10 Thread justinleet
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/1084 +1, once @ottobackwards is good with everything. ---

[GitHub] metron issue #1084: METRON-1644: Support parser chaining

2018-07-10 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 Intermittent test failure, travis sorted now. ---

[GitHub] metron issue #1084: METRON-1644: Support parser chaining

2018-07-06 Thread justinleet
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/1084 I was able to spin this up and run through the README example, with the fix from the most recent commit. Looks like something's going on in Travis, unfortunately. ---

[GitHub] metron issue #1084: METRON-1644: Support parser chaining

2018-07-06 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 Ok, more javadocs is definitely fair. I went through the core abstractions and added javadocs. If I missed anything, let me know. ---

[GitHub] metron issue #1084: METRON-1644: Support parser chaining

2018-07-06 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/1084 Let's try something else. Please javadoc all the new classes and functionality, such that someone else if they want to review or maintain this can understand their implementation ---

[GitHub] metron issue #1084: METRON-1644: Support parser chaining

2018-07-06 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 @ottobackwards Ok, I attempted to do that [here](https://github.com/cestella/incubator-metron/blob/c4e4786e778d5b06cd16f7faa7d3522f620fc2ba/metron-platform/metron-parsers/ParserChaining.md). Can

[GitHub] metron issue #1084: METRON-1644: Support parser chaining

2018-07-06 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/1084 I'm looking for a doc that describes how these new things work, like what would have come out of a discuss thread if there had been discussion on this before hand. " Dev. design

[GitHub] metron issue #1084: METRON-1644: Support parser chaining

2018-07-06 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 Ok, the prerequisite tickets are merged here and this is ready for review. ---

[GitHub] metron issue #1084: METRON-1644: Support parser chaining (NOTE: review METRO...

2018-06-27 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 @ottobackwards Ok, let me know what you think about [this](https://github.com/cestella/incubator-metron/blob/enveloped_message_supplier/metron-platform/metron-parsers/ParserChaining.md). Is it

[GitHub] metron issue #1084: METRON-1644: Support parser chaining (NOTE: review METRO...

2018-06-27 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 @ottobackwards Yeah, that is a really good point. I'll write up something. ---

[GitHub] metron issue #1084: METRON-1644: Support parser chaining (NOTE: review METRO...

2018-06-27 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/1084 Yes. The use case is useful, but this is more dev. focused, if that makes sense. ---

[GitHub] metron issue #1084: METRON-1644: Support parser chaining (NOTE: review METRO...

2018-06-27 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 Yeah, I tend to agree @ottobackwards . There is a high level diagram in the use-case under the "High Level Solution" section; do you think that should be extracted out into its own document along

[GitHub] metron issue #1084: METRON-1644: Support parser chaining (NOTE: review METRO...

2018-06-27 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/1084 This is significant enough that I think some level of design write-up is warranted. At some point we'll want to update the top level doc's and diagrams, but I'm OK with that being a follow