PR Review Request

[Tibor Meller]

Hi all,

Could some of our committers take a look at this? Tamas (ruffle1996)
already left a non-binding +1.
https://github.com/apache/metron/pull/1514

I found a few issue in the master what I would like to fix in the top of
this. (Tickets created and mentioned in the PR desc).

Thanks in advance!

Tibor

Re: [DISCUSS] Alerts UI: Loading state while fetching data

[Tibor Meller]

Ok, I start working on it.

Thanks

On Wed, Jul 24, 2019 at 11:13 PM Michael Miklavcic <
michael.miklav...@gmail.com> wrote:

> +1. I don't think there's really much to discuss, tbh. Definitely
> appreciate your putting this out to the dev list, however, and I'd have
> been just as happy to see it roll in via a PR. This just seems like prudent
> UX design IMO. The sooner we get this, the better.
>
> Cheers,
> Mike
>
> On Wed, Jul 24, 2019 at 8:11 AM Nick Allen  wrote:
>
> > Yes!  I think this is sorely needed.
> >
> > Would this also include indicating when an error has occurred in the
> > backend call?  That might also be helpful and somewhat related to
> > METRON-2190.
> >
> > On Wed, Jul 24, 2019 at 9:27 AM Tibor Meller 
> > wrote:
> >
> > > Hi all,
> > >
> > > I think it would great to have a loading state on the Alerts UI which
> > shows
> > > loading is in progress (spinner) and prevent the user to trigger new
> > fetch
> > > requests before the response arrives (or timeout).
> > >
> > > I added more detail here:
> > > https://issues.apache.org/jira/browse/METRON-2190
> > >
> > > Please take a look at it when you have a chance and let me know what
> you
> > > think.
> > >
> > > Regards,
> > > Tibor
> > >
> >
>

[DISCUSS] Alerts UI: Loading state while fetching data

[Tibor Meller]

Hi all,

I think it would great to have a loading state on the Alerts UI which shows
loading is in progress (spinner) and prevent the user to trigger new fetch
requests before the response arrives (or timeout).

I added more detail here: https://issues.apache.org/jira/browse/METRON-2190

Please take a look at it when you have a chance and let me know what you
think.

Regards,
Tibor

Re: [DISCUSS] Parser Aggregation in Management UI

[Tibor Meller]

HI,
Can we continue the review process with the next PR in the line?
https://github.com/apache/metron/pull/1414

First three PR get merged to the feature branch by Ryan. Thanks for that!

On Tue, Jun 11, 2019 at 10:20 PM Michael Miklavcic <
michael.miklav...@gmail.com> wrote:

> Agreed Ryan, I know this is a bit of a cart before the horse scenario.
>
> This isn't the most desirable of circumstances, and we're doing our best to
> accommodate the contribution because it's a good feature to have, and the
> work so far looks pretty solid. We have a bit more flexibility in a feature
> branch than we would if this was (as it was originally) submitted against
> master - I think the most important thing here is we do
> architecture/code/test review on an individual PR basis. They've split up
> this work pretty fine-grained, so I don't think it should be too hard for
> us to work through. When we get through PRs 1-14 from a code review
> perspective, we'll have to do a run of manual acceptance tests on the final
> product before accepting the feature branch into master. Anything that
> appears to be broken or missing will simply need another PR to address it,
> and we can get it in. I actually don't think this should be too bad. Again,
> thank you to Shane, Tibor, and Tamas for breaking this down. Just be
> careful in the future when you're collaborating on something of this size
> that you either A) split off the work in fully functioning PRs, e.g.
> refactoring work or library changes can very easily be submitted against
> master without breaking anything or B) start with a feature branch and
> discuss thread that lays out your plan for delivering the feature in that
> FB.
>
> Please review Apache's guide on consensus building, as well -
> http://community.apache.org/committers/consensusBuilding.html
>
> On Tue, Jun 11, 2019 at 12:01 PM Ryan Merriman 
> wrote:
>
> > "We planning to add the changes to the latest PR as additional commits to
> > avoid impacting the PR sequence. We will refer to the source PR in the
> > commit message of the fix. Also adding a link to the comment section of
> the
> > source PR of the change request to the fixing commit to make them
> > connected."
> >
> > I don't think this is going to work.  If changes are requested and
> applied
> > in a different PR, how would you test the original PR?  What would you do
> > if there were merge conflicts introduced between the current and final
> PR?
> > What's the point of even having any PRs before the final one if they
> won't
> > get committed or changed?  I don't see any way to avoid having to
> propagate
> > changes through all subsequent PRs.
> >
> > On Wed, May 29, 2019 at 7:03 AM Tibor Meller 
> > wrote:
> >
> > > Hi all,
> > >
> > > *We still need some volunteer reviewers for this feature.* All
> individual
> > > PR is under 1000 line of changes except one and it is due to an
> > > autogenerated package.lock.json file.
> > > Just a heads up: parser aggregation by default turned on for bro, snort
> > and
> > > yarn parser on full dev. Without this changeset, full dev is broken.
> > >
> > >
> >
> https://lists.apache.org/thread.html/beeb4cfddfca7958a22ab926f72f52f46a33c42edce714112df9a2da@%3Cdev.metron.apache.org%3E
> > >
> > >
> > >
> > > On Fri, May 24, 2019 at 3:20 PM Tibor Meller 
> > > wrote:
> > >
> > > > Please find below the list of the PRs we opened for Parser
> Aggregation.
> > > > With Shane, Tamas we tried to provide as much information as possible
> > to
> > > > make the reviewing process easier.
> > > > Please keep that in mind these PRs are not against muster but a
> Parser
> > > > Aggregation feature branch.
> > > > If you like to read more about the process we followed with these PRs
> > > > please read the previous three message in this thread.
> > > >
> > > > PR#1 METRON-2114: [UI] Moving components to sensor parser module
> > > > <https://github.com/apache/metron/pull/1410>
> > > > PR#2 METRON-2116: [UI] Removing redundant AppConfigService
> > > > <https://github.com/apache/metron/pull/1411>
> > > > PR#3 METRON-2117: [UI] Aligning models to grouping feature
> > > > <https://github.com/apache/metron/pull/1412>
> > > > PR#4 METRON-2115: [UI] Aligning UI to the parser aggregation AP
> > > > <https://github.com/apache/metron/pull/1414>
> > > > PR#5 METRON-2122: [UI] Fixing early app config access issue
> > > > &

Re: [DISCUSS] Bug maybe: Alert UI can't use two status filter

[Tibor Meller]

Hi all,

I just opened a PR for fixing filtering in Alerts UI. I found a few other
use cases which haven't worked as expected.
https://github.com/apache/metron/pull/1443

Regards,
Tibor

On Mon, Jun 3, 2019 at 5:13 PM Tibor Meller  wrote:

> Thanks for the answers!
> Ticket created: https://issues.apache.org/jira/browse/METRON-2150
> I start working on it bc it bocking me.
>
> On Fri, May 31, 2019 at 9:43 PM Michael Miklavcic <
> michael.miklav...@gmail.com> wrote:
>
>> Can you log a Jira for this Tibor?
>>
>> On Fri, May 31, 2019 at 1:22 PM Otto Fowler 
>> wrote:
>>
>> > Bug
>> >
>> >
>> > On May 31, 2019 at 08:53:21, Tibor Meller (tibor.mel...@gmail.com)
>> wrote:
>> >
>> > Hi all,
>> >
>> > tl;dr: Is it a bug if "* -alert_status:RESOLVE OR -alert_status:DISMISS"
>> > not works on Alert UI?
>> >
>> > I'm experimenting with filtering from the alert UI. What I found is I
>> can
>> > run the following query directly against the REST API without any
>> problem:
>> > "* -alert_status:RESOLVE OR -alert_status:DISMISS"
>> > But if I try to add the same filter on the Alert UI only the
>> > alert_status:DISMISS applies to the query.
>> > The reason is we identifying filters by filter.field so two filters to
>> the
>> > same filed (alert_status in this case) override each other.
>> >
>> > Is this a bug? If it is, I'm happy to fix it.
>> >
>> > Regards,
>> > Tibor
>> >
>>
>

Re: [DISCUSS] Bug maybe: Alert UI can't use two status filter

[Tibor Meller]

Thanks for the answers!
Ticket created: https://issues.apache.org/jira/browse/METRON-2150
I start working on it bc it bocking me.

On Fri, May 31, 2019 at 9:43 PM Michael Miklavcic <
michael.miklav...@gmail.com> wrote:

> Can you log a Jira for this Tibor?
>
> On Fri, May 31, 2019 at 1:22 PM Otto Fowler 
> wrote:
>
> > Bug
> >
> >
> > On May 31, 2019 at 08:53:21, Tibor Meller (tibor.mel...@gmail.com)
> wrote:
> >
> > Hi all,
> >
> > tl;dr: Is it a bug if "* -alert_status:RESOLVE OR -alert_status:DISMISS"
> > not works on Alert UI?
> >
> > I'm experimenting with filtering from the alert UI. What I found is I can
> > run the following query directly against the REST API without any
> problem:
> > "* -alert_status:RESOLVE OR -alert_status:DISMISS"
> > But if I try to add the same filter on the Alert UI only the
> > alert_status:DISMISS applies to the query.
> > The reason is we identifying filters by filter.field so two filters to
> the
> > same filed (alert_status in this case) override each other.
> >
> > Is this a bug? If it is, I'm happy to fix it.
> >
> > Regards,
> > Tibor
> >
>

Re: [DISCUSS] Click through navigation feature

[Tibor Meller]

Thanks, Michael for your feedback! I had somewhat similar questions in the
back of my mind. I'm going to think them through and come back to you.
Thanks for opening the discussion on feature switches. There are a lot of
ways to approach this and would be great to hear any ideas from the
community.

On Fri, May 31, 2019 at 7:28 PM Michael Miklavcic <
michael.miklav...@gmail.com> wrote:

> Thank you Tibor - I've made some comments on your PR here -
> https://github.com/apache/metron/pull/1431#issuecomment-497794088
>
> On Fri, May 31, 2019 at 8:56 AM Tibor Meller 
> wrote:
>
> > Hi all,
> >
> > I build a feature for Alert UI, what we found very useful and I think it
> > would be a great candidate for a PR.
> >
> > Short Description:
> > Click Through Navigation is a feature makes Metron Users able to reach
> > other web services via dynamically created URLs by clicking link item in
> a
> > context menu.
> > This context menu (aka. click-through menu) is attached to the alerts
> table
> > and the links are populated with alert data from the specific row of the
> > table.
> >
> > Because probably not all user like to integrate the Alerts UI with other
> > services in his/her system the feature is optional. Users have to
> > intentionally activate the context menu. Otherwise, the current
> > functionality of the alerts table doesn't change.
> >
> > You can find more about this in the following link:
> >
> >
> https://issues.apache.org/jira/projects/METRON/issues/METRON-2102?filter=allopenissues
> > and PR:
> > https://github.com/apache/metron/pull/1431
> >
> > Regards,
> > Tibor
> >
>

[DISCUSS] Click through navigation feature

[Tibor Meller]

Hi all,

I build a feature for Alert UI, what we found very useful and I think it
would be a great candidate for a PR.

Short Description:
Click Through Navigation is a feature makes Metron Users able to reach
other web services via dynamically created URLs by clicking link item in a
context menu.
This context menu (aka. click-through menu) is attached to the alerts table
and the links are populated with alert data from the specific row of the
table.

Because probably not all user like to integrate the Alerts UI with other
services in his/her system the feature is optional. Users have to
intentionally activate the context menu. Otherwise, the current
functionality of the alerts table doesn't change.

You can find more about this in the following link:
https://issues.apache.org/jira/projects/METRON/issues/METRON-2102?filter=allopenissues
and PR:
https://github.com/apache/metron/pull/1431

Regards,
Tibor

[DISCUSS] Bug maybe: Alert UI can't use two status filter

[Tibor Meller]

Hi all,

tl;dr: Is it a bug if "* -alert_status:RESOLVE OR -alert_status:DISMISS"
not works on Alert UI?

I'm experimenting with filtering from the alert UI. What I found is I can
run the following query directly against the REST API without any problem:
"* -alert_status:RESOLVE OR -alert_status:DISMISS"
But if I try to add the same filter on the Alert UI only the
alert_status:DISMISS applies to the query.
The reason is we identifying filters by filter.field so two filters to the
same filed (alert_status in this case) override each other.

Is this a bug? If it is, I'm happy to fix it.

Regards,
Tibor

Re: [DISCUSS] Parser Aggregation in Management UI

[Tibor Meller]

Hi all,

*We still need some volunteer reviewers for this feature.* All individual
PR is under 1000 line of changes except one and it is due to an
autogenerated package.lock.json file.
Just a heads up: parser aggregation by default turned on for bro, snort and
yarn parser on full dev. Without this changeset, full dev is broken.
https://lists.apache.org/thread.html/beeb4cfddfca7958a22ab926f72f52f46a33c42edce714112df9a2da@%3Cdev.metron.apache.org%3E



On Fri, May 24, 2019 at 3:20 PM Tibor Meller  wrote:

> Please find below the list of the PRs we opened for Parser Aggregation.
> With Shane, Tamas we tried to provide as much information as possible to
> make the reviewing process easier.
> Please keep that in mind these PRs are not against muster but a Parser
> Aggregation feature branch.
> If you like to read more about the process we followed with these PRs
> please read the previous three message in this thread.
>
> PR#1 METRON-2114: [UI] Moving components to sensor parser module
> <https://github.com/apache/metron/pull/1410>
> PR#2 METRON-2116: [UI] Removing redundant AppConfigService
> <https://github.com/apache/metron/pull/1411>
> PR#3 METRON-2117: [UI] Aligning models to grouping feature
> <https://github.com/apache/metron/pull/1412>
> PR#4 METRON-2115: [UI] Aligning UI to the parser aggregation AP
> <https://github.com/apache/metron/pull/1414>
> PR#5 METRON-2122: [UI] Fixing early app config access issue
> <https://github.com/apache/metron/pull/1415>
> PR#6 METRON-2124: [UI] Move status information and start/stop to the
> Aggregate level <https://github.com/apache/metron/pull/1418>
> PR#7 METRON-2125: [UI] Making changes visible in the parser list by
> marking changed items <https://github.com/apache/metron/pull/1422>
> PR#8 METRON-2131: Add NgRx and related dependencies
> <https://github.com/apache/metron/pull/1423>
> PR#9 METRON-2133: Add NgRx effects to communicate with the server
> <https://github.com/apache/metron/pull/1424>
> PR#10 METRON-2134: Add NgRx reducers to perform parser and group changes
> in the store <https://github.com/apache/metron/pull/1425>
> PR#11 METRON-2135: Add NgRx actions to trigger state changes
> <https://github.com/apache/metron/pull/1426>
> PR#12 METRON-2136: Add parser aggregation sidebar
> <https://github.com/apache/metron/pull/1427>
> PR#13 METRON-2137: Implement drag and drop mechanism and wire NgRx
> <https://github.com/apache/metron/pull/1428>
> PR#14 METRON-2138: Code clean up
> <https://github.com/apache/metron/pull/1429>
> PR#15 METRON-2139: Refactoring sensor-parser-config.component and wire
> NgRx <https://github.com/apache/metron/pull/1430>
>
> Thanks,
> Tibor
>
>
> On Thu, May 23, 2019 at 11:45 AM Tibor Meller 
> wrote:
>
>> Yes, am expecting that some change request will rase due to the review.
>> We planning to add the changes to the latest PR as additional commits to
>> avoid impacting the PR sequence. We will refer to the source PR in the
>> commit message of the fix. Also adding a link to the comment section of the
>> source PR of the change request to the fixing commit to make them connected.
>>
>> On Wed, May 22, 2019 at 5:49 PM Michael Miklavcic <
>> michael.miklav...@gmail.com> wrote:
>>
>>> Tibor, that sounds reasonable to me. If PR #1 ends up requiring code
>>> changes, will you guys just percolate those up through the remaining k
>>> PRs
>>> in order, or just the final PR? I'm wondering how this works in reference
>>> to your last point in #5 about rebasing.
>>>
>>> On Wed, May 22, 2019 at 8:47 AM Tibor Meller 
>>> wrote:
>>>
>>> > I would like to describe quickly *our approach to breaking down Parser
>>> > Aggregation PR for smaller chunks*
>>> >
>>> > *1. we squashed the commits in the original development branch*
>>> > - when we started to open smaller PRs from the commits from the
>>> original
>>> > branch, we found ourself opening PRs out of historical states of the
>>> code
>>> > instead of the final one
>>> > - none of those states of development are worth (or make sense) to be
>>> > reviewed (initial phases of development are included in the original
>>> commit
>>> > history, multiple iterations of refactoring, etc.)
>>> > - while the actual development history was irrelevant, the attribution
>>> > aspect of it was still important
>>> > *2. we divided** the changes by the original authors*
>>> > - the original contributors were sardell, ruffle1986 and tiborm
>>> > - we 

Re: [DISCUSS] Parser Aggregation in Management UI

[Tibor Meller]

Please find below the list of the PRs we opened for Parser Aggregation.
With Shane, Tamas we tried to provide as much information as possible to
make the reviewing process easier.
Please keep that in mind these PRs are not against muster but a Parser
Aggregation feature branch.
If you like to read more about the process we followed with these PRs
please read the previous three message in this thread.

PR#1 METRON-2114: [UI] Moving components to sensor parser module
<https://github.com/apache/metron/pull/1410>
PR#2 METRON-2116: [UI] Removing redundant AppConfigService
<https://github.com/apache/metron/pull/1411>
PR#3 METRON-2117: [UI] Aligning models to grouping feature
<https://github.com/apache/metron/pull/1412>
PR#4 METRON-2115: [UI] Aligning UI to the parser aggregation AP
<https://github.com/apache/metron/pull/1414>
PR#5 METRON-2122: [UI] Fixing early app config access issue
<https://github.com/apache/metron/pull/1415>
PR#6 METRON-2124: [UI] Move status information and start/stop to the
Aggregate level <https://github.com/apache/metron/pull/1418>
PR#7 METRON-2125: [UI] Making changes visible in the parser list by marking
changed items <https://github.com/apache/metron/pull/1422>
PR#8 METRON-2131: Add NgRx and related dependencies
<https://github.com/apache/metron/pull/1423>
PR#9 METRON-2133: Add NgRx effects to communicate with the server
<https://github.com/apache/metron/pull/1424>
PR#10 METRON-2134: Add NgRx reducers to perform parser and group changes in
the store <https://github.com/apache/metron/pull/1425>
PR#11 METRON-2135: Add NgRx actions to trigger state changes
<https://github.com/apache/metron/pull/1426>
PR#12 METRON-2136: Add parser aggregation sidebar
<https://github.com/apache/metron/pull/1427>
PR#13 METRON-2137: Implement drag and drop mechanism and wire NgRx
<https://github.com/apache/metron/pull/1428>
PR#14 METRON-2138: Code clean up
<https://github.com/apache/metron/pull/1429>
PR#15 METRON-2139: Refactoring sensor-parser-config.component and wire NgRx
<https://github.com/apache/metron/pull/1430>

Thanks,
Tibor


On Thu, May 23, 2019 at 11:45 AM Tibor Meller 
wrote:

> Yes, am expecting that some change request will rase due to the review.
> We planning to add the changes to the latest PR as additional commits to
> avoid impacting the PR sequence. We will refer to the source PR in the
> commit message of the fix. Also adding a link to the comment section of the
> source PR of the change request to the fixing commit to make them connected.
>
> On Wed, May 22, 2019 at 5:49 PM Michael Miklavcic <
> michael.miklav...@gmail.com> wrote:
>
>> Tibor, that sounds reasonable to me. If PR #1 ends up requiring code
>> changes, will you guys just percolate those up through the remaining k PRs
>> in order, or just the final PR? I'm wondering how this works in reference
>> to your last point in #5 about rebasing.
>>
>> On Wed, May 22, 2019 at 8:47 AM Tibor Meller 
>> wrote:
>>
>> > I would like to describe quickly *our approach to breaking down Parser
>> > Aggregation PR for smaller chunks*
>> >
>> > *1. we squashed the commits in the original development branch*
>> > - when we started to open smaller PRs from the commits from the original
>> > branch, we found ourself opening PRs out of historical states of the
>> code
>> > instead of the final one
>> > - none of those states of development are worth (or make sense) to be
>> > reviewed (initial phases of development are included in the original
>> commit
>> > history, multiple iterations of refactoring, etc.)
>> > - while the actual development history was irrelevant, the attribution
>> > aspect of it was still important
>> > *2. we divided** the changes by the original authors*
>> > - the original contributors were sardell, ruffle1986 and tiborm
>> > - we isolated the changes that belong to each individual contributor
>> > *3. each of us identified smaller but belonging changesets *
>> > - with this, we ended up opening 5 PRs from tiborm, 3 from sardell and 6
>> > from ruffle1986
>> > - each of these are smaller than 500 lines of changes, which makes the
>> task
>> > of reviewing easier
>> > - they have their own context and purpose described by the PR and the
>> > related Jira ticket
>> > *4. Each PR introduces a single new commit which is meant to be
>> reviewed*
>> > - with this we were able to open PRs on top of each others work, but the
>> > reviewer is still able to identify what changes were introduced and
>> > described by the pr simply by focusing on the last commit
>> > - the commit introduced by the 

Re: [DISCUSS] Parser Aggregation in Management UI

[Tibor Meller]

Yes, am expecting that some change request will rase due to the review.
We planning to add the changes to the latest PR as additional commits to
avoid impacting the PR sequence. We will refer to the source PR in the
commit message of the fix. Also adding a link to the comment section of the
source PR of the change request to the fixing commit to make them connected.

On Wed, May 22, 2019 at 5:49 PM Michael Miklavcic <
michael.miklav...@gmail.com> wrote:

> Tibor, that sounds reasonable to me. If PR #1 ends up requiring code
> changes, will you guys just percolate those up through the remaining k PRs
> in order, or just the final PR? I'm wondering how this works in reference
> to your last point in #5 about rebasing.
>
> On Wed, May 22, 2019 at 8:47 AM Tibor Meller 
> wrote:
>
> > I would like to describe quickly *our approach to breaking down Parser
> > Aggregation PR for smaller chunks*
> >
> > *1. we squashed the commits in the original development branch*
> > - when we started to open smaller PRs from the commits from the original
> > branch, we found ourself opening PRs out of historical states of the code
> > instead of the final one
> > - none of those states of development are worth (or make sense) to be
> > reviewed (initial phases of development are included in the original
> commit
> > history, multiple iterations of refactoring, etc.)
> > - while the actual development history was irrelevant, the attribution
> > aspect of it was still important
> > *2. we divided** the changes by the original authors*
> > - the original contributors were sardell, ruffle1986 and tiborm
> > - we isolated the changes that belong to each individual contributor
> > *3. each of us identified smaller but belonging changesets *
> > - with this, we ended up opening 5 PRs from tiborm, 3 from sardell and 6
> > from ruffle1986
> > - each of these are smaller than 500 lines of changes, which makes the
> task
> > of reviewing easier
> > - they have their own context and purpose described by the PR and the
> > related Jira ticket
> > *4. Each PR introduces a single new commit which is meant to be reviewed*
> > - with this we were able to open PRs on top of each others work, but the
> > reviewer is still able to identify what changes were introduced and
> > described by the pr simply by focusing on the last commit
> > - the commit introduced by the PR has the same commit message as the
> title
> > of the PR to make it easier to find
> > *5. Only the last PR is meant to be merged into the feature branch*
> > - the last PR also introduces a single new commit to being reviewed
> > - this contains all the commits from the previous PRs that belong to
> parser
> > aggregation
> > - it builds fine in Travis
> > - it's fully functional and ready to being tested against full dev
> > - If we only merge the last PR, we don't have to rebase and recreate all
> of
> > our PRs due to merge conflicts that will result from to conflicting
> > histories (which is common in feature branch work)
> >
> > Once all the Pull Requests are open, I will submit a list of all of them
> to
> > this discussion thread.
> >
> > On Wed, May 8, 2019 at 3:58 PM Otto Fowler 
> > wrote:
> >
> > > You need to be a committer, that is all I think.
> > > I would not use the github UI for it though, I do it through the cli
> > >
> > >
> > >
> > > On May 8, 2019 at 09:45:24, Michael Miklavcic (
> > michael.miklav...@gmail.com
> > > )
> > > wrote:
> > >
> > > Not that I'm aware of. Nick and Otto, you've created them before, did
> you
> > > need any special perms?
> > >
> > > On Wed, May 8, 2019 at 3:57 AM Shane Ardell 
> > > wrote:
> > >
> > > > This morning, we started to break down our work as Michael suggested
> in
> > > > this thread. However, it looks like I don't have permission to
> create a
> > > new
> > > > branch in the GitHub UI or push a new branch to the apache/metron
> repo.
> > > Is
> > > > this action restricted to PMC members only?
> > > >
> > > > Shane
> > > >
> > > > On Wed, May 8, 2019 at 9:06 AM Tamás Fodor 
> > > wrote:
> > > >
> > > > > Here's the process we've gone through in order to implement the
> > > feature.
> > > > >
> > > > > At the beginning we had some bootstrap work like creating a mock
> API
> > > > > (written in NodeJS) because we were a few steps ahead the

Re: [DISCUSS] Parser Aggregation in Management UI

[Tibor Meller]

he restriction because it's not intuitive and
> very
> > > inconvenient. In my opinion, we should let the users to aggregate the
> > > running parsers together and do the job to handle this edge case on the
> > > backend accordingly.
> > >
> > > What do you think, guys?
> > >
> > > Hope this helps.
> > >
> > > Tamas
> > >
> > > On Tue, May 7, 2019 at 4:34 PM Michael Miklavcic <
> > > michael.miklav...@gmail.com> wrote:
> > >
> > > > This was my expectation as well.
> > > >
> > > > Shane, Tibor, Tamas - how did you go about breaking this down into
> > chunks
> > > > and/or microchunks when you collaborated offline? As Nick mentioned,
> > you
> > > > obviously split up work and shared it amongst yourselves. Some
> > > explanation
> > > > around this process would be helpful for reviewers as well. We might
> be
> > > > able to provide better guidance and examples to future contributors
> as
> > > > well.
> > > >
> > > > I talked a little bit with Shane about this offline last week. It
> looks
> > > > like you guys effectively ran a local feature branch. Replicating
> that
> > > > process in a feature branch in Apache is probably what you guys
> should
> > > > be doing for a change this size. We don't have hard limits on line
> > change
> > > > size, but in the past it's been somewhere around 2k-3k lines and
> above
> > > > being the tipping point for discussing a feature branch. Strictly
> > > speaking,
> > > > line quantity alone is not the only metric, but it's relevant here.
> If
> > > you
> > > > want to make smaller incremental changes locally, there's nothing to
> > keep
> > > > you from doing that - I would only advise that you consider squashing
> > > those
> > > > commits (just ask if you're unclear about how to handle that) into a
> > > single
> > > > larger commit/chunk when you're ready to publish them as a chunk to
> the
> > > > public feature branch. So it would look something like this:
> > > >
> > > > Commits by person locally
> > > > Shane: 1,2,3 -> squash as A
> > > > Tibor: 4,5,6 -> squash as B
> > > > Tamas: 7,8,9 -> squash as C
> > > >
> > > > Commits by person in Apache
> > > > Shane: A
> > > > Tibor: B
> > > > Tamas: C
> > > >
> > > > We need to maintain a record of attribution. Your real workflow may
> not
> > > be
> > > > that cleanly delineated, but you can choose how you want to squash in
> > > those
> > > > cases. Even in public collaboration, there are plenty of cases where
> > > folks
> > > > submit PRs against PRs, abstain from accepting attribution, and it
> all
> > > gets
> > > > squashed down into one person's final PR commit. There are many
> > options.
> > > >
> > > > Hope this helps.
> > > >
> > > > Best,
> > > > Mike
> > > >
> > > > On Mon, May 6, 2019 at 8:19 AM Nick Allen 
> wrote:
> > > >
> > > > > Have you considered creating a feature branch for the effort? This
> > > would
> > > > > allow you to break the effort into chunks, where the result of each
> > PR
> > > > may
> > > > > not be a fully working "master-ready" result.
> > > > >
> > > > > I am sure you guys tackled the work in chunks when developing it,
> so
> > > > > consider just replaying those chunks onto the feature branch as
> > > separate
> > > > > PRs.
> > > > >
> > > > >
> > > > >
> > > > > On Mon, May 6, 2019 at 5:24 AM Tibor Meller <
> tibor.mel...@gmail.com>
>
> > > > > wrote:
> > > > >
> > > > > > I wondered on the weekend how we could split that PR to smaller
> > > chunks.
> > > > > > That PR is a result of almost 2 months of development and I don't
> > see
> > > > how
> > > > > > to split that to multiple WORKING parts. It is as it is a whole
> > > working
> > > > > > feature. If we split it by packages or files we could provide
> > smaller
> > > > > > non-functional PR's, but can end up having a broken Management UI
> 

Re: [DISCUSS] Parser Aggregation in Management UI

[Tibor Meller]

Thanks Nick! That actually sounds promising.
Ryan are you ok with that if we open multiple smaller PR against the
Feature Branch you planning to create?


On Mon, May 6, 2019 at 4:19 PM Nick Allen  wrote:

> Have you considered creating a feature branch for the effort? This would
> allow you to break the effort into chunks, where the result of each PR may
> not be a fully working "master-ready" result.
>
> I am sure you guys tackled the work in chunks when developing it, so
> consider just replaying those chunks onto the feature branch as separate
> PRs.
>
>
>
> On Mon, May 6, 2019 at 5:24 AM Tibor Meller 
> wrote:
>
> > I wondered on the weekend how we could split that PR to smaller chunks.
> > That PR is a result of almost 2 months of development and I don't see how
> > to split that to multiple WORKING parts. It is as it is a whole working
> > feature. If we split it by packages or files we could provide smaller
> > non-functional PR's, but can end up having a broken Management UI after
> > having the 1st PR part merged into master. I don't think that would be
> > acceptable by the community (or even by me) so I would like to suggest
> two
> > other option to help review PR#1360.
> >
> > #1 We could extend that PR with our own author comments in Github. That
> > would help following which code part belongs to where and why it was
> > necessary.
> > #2 We can schedule an interactive code walkthrough call with the ones who
> > interested in reviewing or the particular changeset.
> >
> > Please share your thoughts on this! Which version would support you the
> > best? Or if you have any other idea let us know.
> >
> > PS: I think the size of our PR's depends on how small independently
> > deliverable changesets we can identify before we starting to implement a
> > relatively big new feature. Unfortunately, we missed to do that with this
> > feature.
> >
> > On Fri, May 3, 2019 at 1:49 PM Shane Ardell 
> > wrote:
> >
> > > NgRx was only used for the aggregation feature and doesn't go beyond
> > that.
> > > I think the way I worded that sentence may have caused confusion. I
> just
> > > meant we use it to manage more pieces of state within the aggregation
> > > feature than just previous and current state of grouped parsers.
> > >
> > > On Fri, May 3, 2019 at 1:32 AM Michael Miklavcic <
> > > michael.miklav...@gmail.com> wrote:
> > >
> > > > Shane, thanks for putting this together. The updates on the Jira are
> > > useful
> > > > as well.
> > > >
> > > > > (we used it for more than just that in this feature, but that was
> the
> > > > initial reasoning)
> > > > What are you using NgRx for in the submitted work that goes beyond
> the
> > > > aggregation feature?
> > > >
> > > >
> > > >
> > > > On Thu, May 2, 2019 at 12:22 PM Shane Ardell <
> shane.m.ard...@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > Hello everyone,
> > > > >
> > > > > In response to discussions in the 0.7.1 release thread, I wanted to
> > > > start a
> > > > > thread regarding the parser aggregation work for the Management UI.
> > For
> > > > > anyone who has not already read and tested the PR locally, I've
> > added a
> > > > > detailed description of what we did and why to the JIRA ticket
> here:
> > > > > https://issues.apache.org/jira/browse/METRON-1856
> > > > >
> > > > > I'm wondering what the community thinks about what we've built thus
> > > far.
> > > > Do
> > > > > you see anything missing that must be part of this new feature in
> the
> > > UI?
> > > > > Are there any strong objections to how we implemented it?
> > > > >
> > > > > I’m also looking to see if anyone has any thoughts on how we can
> > > possibly
> > > > > simplify this PR. Right now it's pretty big, and there are a lot of
> > > > commits
> > > > > to parse through, but I'm not sure how we could break this work out
> > > into
> > > > > separate, smaller PRs opened against master. We could try to
> > > cherry-pick
> > > > > the commits into smaller PRs and then merge them into a feature
> > branch,
> > > > but
> > > > > I'm not sure if that's worth the effort since that will only reduce
> > the
> >

Re: [DISCUSS] Parser Aggregation in Management UI

[Tibor Meller]

@Otto: I responded to your questions in a few Jira comment.

On Mon, May 6, 2019 at 11:21 AM Tibor Meller  wrote:

> I wondered on the weekend how we could split that PR to smaller chunks.
> That PR is a result of almost 2 months of development and I don't see how
> to split that to multiple WORKING parts. It is as it is a whole working
> feature. If we split it by packages or files we could provide smaller
> non-functional PR's, but can end up having a broken Management UI after
> having the 1st PR part merged into master. I don't think that would be
> acceptable by the community (or even by me) so I would like to suggest two
> other option to help review PR#1360.
>
> #1 We could extend that PR with our own author comments in Github. That
> would help following which code part belongs to where and why it was
> necessary.
> #2 We can schedule an interactive code walkthrough call with the ones who
> interested in reviewing or the particular changeset.
>
> Please share your thoughts on this! Which version would support you the
> best? Or if you have any other idea let us know.
>
> PS: I think the size of our PR's depends on how small independently
> deliverable changesets we can identify before we starting to implement a
> relatively big new feature. Unfortunately, we missed to do that with this
> feature.
>
> On Fri, May 3, 2019 at 1:49 PM Shane Ardell 
> wrote:
>
>> NgRx was only used for the aggregation feature and doesn't go beyond that.
>> I think the way I worded that sentence may have caused confusion. I just
>> meant we use it to manage more pieces of state within the aggregation
>> feature than just previous and current state of grouped parsers.
>>
>> On Fri, May 3, 2019 at 1:32 AM Michael Miklavcic <
>> michael.miklav...@gmail.com> wrote:
>>
>> > Shane, thanks for putting this together. The updates on the Jira are
>> useful
>> > as well.
>> >
>> > > (we used it for more than just that in this feature, but that was the
>> > initial reasoning)
>> > What are you using NgRx for in the submitted work that goes beyond the
>> > aggregation feature?
>> >
>> >
>> >
>> > On Thu, May 2, 2019 at 12:22 PM Shane Ardell 
>> > wrote:
>> >
>> > > Hello everyone,
>> > >
>> > > In response to discussions in the 0.7.1 release thread, I wanted to
>> > start a
>> > > thread regarding the parser aggregation work for the Management UI.
>> For
>> > > anyone who has not already read and tested the PR locally, I've added
>> a
>> > > detailed description of what we did and why to the JIRA ticket here:
>> > > https://issues.apache.org/jira/browse/METRON-1856
>> > >
>> > > I'm wondering what the community thinks about what we've built thus
>> far.
>> > Do
>> > > you see anything missing that must be part of this new feature in the
>> UI?
>> > > Are there any strong objections to how we implemented it?
>> > >
>> > > I’m also looking to see if anyone has any thoughts on how we can
>> possibly
>> > > simplify this PR. Right now it's pretty big, and there are a lot of
>> > commits
>> > > to parse through, but I'm not sure how we could break this work out
>> into
>> > > separate, smaller PRs opened against master. We could try to
>> cherry-pick
>> > > the commits into smaller PRs and then merge them into a feature
>> branch,
>> > but
>> > > I'm not sure if that's worth the effort since that will only reduce
>> the
>> > > number commits to review, not the lines changed.
>> > >
>> > > As an aside, I also want to give a little background into the
>> > introduction
>> > > of NgRx in this PR. To give a little background on why we chose to do
>> > this,
>> > > you can refer to the discussion thread here:
>> > >
>> > >
>> >
>> https://lists.apache.org/thread.html/06a59ea42e8d9a9dea5f90aab4011e44434555f8b7f3cf21297c7c87@%3Cdev.metron.apache.org%3E
>> > >
>> > > We previously discussed introducing a better way to manage application
>> > > state in both UIs in that thread. It was decided that NgRx was a great
>> > tool
>> > > for many reasons, one of them being that we can piecemeal it into the
>> > > application rather than doing a huge rewrite of all the application
>> state
>> > > at once. The contributors in this PR (myself included) decided this
>> would
>> >

Re: [DISCUSS] Parser Aggregation in Management UI

[Tibor Meller]

I wondered on the weekend how we could split that PR to smaller chunks.
That PR is a result of almost 2 months of development and I don't see how
to split that to multiple WORKING parts. It is as it is a whole working
feature. If we split it by packages or files we could provide smaller
non-functional PR's, but can end up having a broken Management UI after
having the 1st PR part merged into master. I don't think that would be
acceptable by the community (or even by me) so I would like to suggest two
other option to help review PR#1360.

#1 We could extend that PR with our own author comments in Github. That
would help following which code part belongs to where and why it was
necessary.
#2 We can schedule an interactive code walkthrough call with the ones who
interested in reviewing or the particular changeset.

Please share your thoughts on this! Which version would support you the
best? Or if you have any other idea let us know.

PS: I think the size of our PR's depends on how small independently
deliverable changesets we can identify before we starting to implement a
relatively big new feature. Unfortunately, we missed to do that with this
feature.

On Fri, May 3, 2019 at 1:49 PM Shane Ardell 
wrote:

> NgRx was only used for the aggregation feature and doesn't go beyond that.
> I think the way I worded that sentence may have caused confusion. I just
> meant we use it to manage more pieces of state within the aggregation
> feature than just previous and current state of grouped parsers.
>
> On Fri, May 3, 2019 at 1:32 AM Michael Miklavcic <
> michael.miklav...@gmail.com> wrote:
>
> > Shane, thanks for putting this together. The updates on the Jira are
> useful
> > as well.
> >
> > > (we used it for more than just that in this feature, but that was the
> > initial reasoning)
> > What are you using NgRx for in the submitted work that goes beyond the
> > aggregation feature?
> >
> >
> >
> > On Thu, May 2, 2019 at 12:22 PM Shane Ardell 
> > wrote:
> >
> > > Hello everyone,
> > >
> > > In response to discussions in the 0.7.1 release thread, I wanted to
> > start a
> > > thread regarding the parser aggregation work for the Management UI. For
> > > anyone who has not already read and tested the PR locally, I've added a
> > > detailed description of what we did and why to the JIRA ticket here:
> > > https://issues.apache.org/jira/browse/METRON-1856
> > >
> > > I'm wondering what the community thinks about what we've built thus
> far.
> > Do
> > > you see anything missing that must be part of this new feature in the
> UI?
> > > Are there any strong objections to how we implemented it?
> > >
> > > I’m also looking to see if anyone has any thoughts on how we can
> possibly
> > > simplify this PR. Right now it's pretty big, and there are a lot of
> > commits
> > > to parse through, but I'm not sure how we could break this work out
> into
> > > separate, smaller PRs opened against master. We could try to
> cherry-pick
> > > the commits into smaller PRs and then merge them into a feature branch,
> > but
> > > I'm not sure if that's worth the effort since that will only reduce the
> > > number commits to review, not the lines changed.
> > >
> > > As an aside, I also want to give a little background into the
> > introduction
> > > of NgRx in this PR. To give a little background on why we chose to do
> > this,
> > > you can refer to the discussion thread here:
> > >
> > >
> >
> https://lists.apache.org/thread.html/06a59ea42e8d9a9dea5f90aab4011e44434555f8b7f3cf21297c7c87@%3Cdev.metron.apache.org%3E
> > >
> > > We previously discussed introducing a better way to manage application
> > > state in both UIs in that thread. It was decided that NgRx was a great
> > tool
> > > for many reasons, one of them being that we can piecemeal it into the
> > > application rather than doing a huge rewrite of all the application
> state
> > > at once. The contributors in this PR (myself included) decided this
> would
> > > be a perfect opportunity to introduce NgRx into the Management UI since
> > we
> > > need to manage the previous and current state with the grouping feature
> > so
> > > that users can undo the changes they've made (we used it for more than
> > just
> > > that in this feature, but that was the initial reasoning). In addition,
> > we
> > > greatly benefited from this when it came time to debug our work in the
> UI
> > > (the discussion in the above thread link goes a little more into the
> > > advantages of debugging with NgRx and DevTools). Removing NgRx from
> this
> > > work would reduce the numbers of lines changed slightly, but it would
> > still
> > > be a big PR and a lot of that code would just move to the component or
> > > service level in the Angular application.
> > >
> > > Shane
> > >
> >
>

Re: [DISCUSS] Metron Release - 0.7.1 next steps

[Tibor Meller]

A separate [DISCUSSION] thread on Parser Aggregation support for the
Management UI is coming later today.
We collecting all the previous threads there which belongs to this feature
and it's implementation.

On Thu, May 2, 2019 at 6:32 PM Tibor Meller  wrote:

> I also favor option 2. I also feel it is good to highlight we are not
> facing with an issue on the UI with a fix in PR#1360.
> Parser aggregation had turned on for the three default parser without
> having parser aggregation support added to the UI.
> PR#1360 contains a whole new feature with about 6000 lines of code
> changes. Which I think hold a fair amount of risk for regression.
> I suggest considering this PR more than a simple patch for the parser
> issue introduced in our previous release.
> This is probably the biggest feature on the UI in the last one year.
> Therefore am not even sure it belongs to a patch release like 0.7.x instead
> of 0.8.0.
> The latest version of the REST API with parser aggregation just came out
> yesterday so we were able to start another round of testing.
> I already identified three minor bugs. Some of them (if not all) have to
> be fixed before we can consider this PR done.
> Long story short: am also against the pressure to pushing this PR out ASAP.
>
> On Thu, May 2, 2019 at 5:50 PM Michael Miklavcic <
> michael.miklav...@gmail.com> wrote:
>
>> As a more general question, can I ask why we're feeling pressure to push
>> out a release in the first place? Again, I'm happy to continue with option
>> 2. Let's move forward and get out the release. But is there a reason why
>> we
>> think it has to get out now, versus next week, or the week after? Otto
>> pointed out a legitimate issue, dev environment or not, and I'm unclear
>> why
>> we have an issue with waiting for the fix. There's no pressure on this,
>> imho.
>>
>> On Thu, May 2, 2019, 9:12 AM Otto Fowler  wrote:
>>
>> > I remember this now, but I’m not sure how I would have related this to a
>> > parser aggregation pr honestly.
>> >
>> >
>> > On May 2, 2019 at 07:54:13, Shane Ardell (shane.m.ard...@gmail.com)
>> wrote:
>> >
>> > Here's a link to the ngrx discussion thread from a few months back:
>> >
>> >
>> https://lists.apache.org/thread.html/06a59ea42e8d9a9dea5f90aab4011e44434555f8b7f3cf21297c7c87@%3Cdev.metron.apache.org%3E
>> >
>> > On Thu, May 2, 2019 at 1:17 PM Otto Fowler 
>> > wrote:
>> >
>> > > If you can find a link in the archives for that thread, it would
>> really
>> > > help.
>> > >
>> > > I don’t think sending them up as one sensor would work…. as something
>> > > quick. I think it is an interesting idea from a higher level that
>> would
>> > > need some more thought though ( IE: what if every sensor in the ui
>> was a
>> > > sensor group, and the existing entries where just groups of 1 ).
>> > >
>> > > As far as I can see, we have brought up the idea of a release
>> ourselves,
>> > I
>> > > don’t see why we don’t just swarm this issue and get it right then
>> > release.
>> > >
>> > >
>> > >
>> > > On May 2, 2019 at 04:16:31, Tamás Fodor (ftamas.m...@gmail.com)
>> wrote:
>> > >
>> > > In PR#1360 we introduced a new state management strategy involving a
>> new
>> > > module called Ngrx. We had a discussion thread on this a few months
>> ago
>> > and
>> > > we successfully convinced you about the benefits. This is one of the
>> > > reasons why this PR is going to be still huge after cleaning up the
>> > commit
>> > > history. After you having a look at the changes and the feature
>> itself,
>> > > there's likely have questions about why certain parts work as they do.
>> > The
>> > > thing what I'd like to point out is that, yes, it probably takes more
>> > time
>> > > to get it in.
>> > >
>> > > In order to being able to release the RC, wouldn't it be an easy and
>> > quick
>> > > fix on the backend if it sent the aggregated parsers to the client as
>> > they
>> > > were one sensor? It's just an idea, it might be wrong, but at least we
>> > > shouldn't have to wait until the aforementioned PR gets ready to be
>> > merged
>> > > to the master.
>> > >
>> > > On Wed, May 1, 2019 at 4:16 PM Justin Leet 
>> > wrote:
>> > >
>> > > > Short version: I'm in 

Re: [DISCUSS] Metron Release - 0.7.1 next steps

[Tibor Meller]

I also favor option 2. I also feel it is good to highlight we are not
facing with an issue on the UI with a fix in PR#1360.
Parser aggregation had turned on for the three default parser without
having parser aggregation support added to the UI.
PR#1360 contains a whole new feature with about 6000 lines of code changes.
Which I think hold a fair amount of risk for regression.
I suggest considering this PR more than a simple patch for the parser issue
introduced in our previous release.
This is probably the biggest feature on the UI in the last one year.
Therefore am not even sure it belongs to a patch release like 0.7.x instead
of 0.8.0.
The latest version of the REST API with parser aggregation just came out
yesterday so we were able to start another round of testing.
I already identified three minor bugs. Some of them (if not all) have to be
fixed before we can consider this PR done.
Long story short: am also against the pressure to pushing this PR out ASAP.

On Thu, May 2, 2019 at 5:50 PM Michael Miklavcic <
michael.miklav...@gmail.com> wrote:

> As a more general question, can I ask why we're feeling pressure to push
> out a release in the first place? Again, I'm happy to continue with option
> 2. Let's move forward and get out the release. But is there a reason why we
> think it has to get out now, versus next week, or the week after? Otto
> pointed out a legitimate issue, dev environment or not, and I'm unclear why
> we have an issue with waiting for the fix. There's no pressure on this,
> imho.
>
> On Thu, May 2, 2019, 9:12 AM Otto Fowler  wrote:
>
> > I remember this now, but I’m not sure how I would have related this to a
> > parser aggregation pr honestly.
> >
> >
> > On May 2, 2019 at 07:54:13, Shane Ardell (shane.m.ard...@gmail.com)
> wrote:
> >
> > Here's a link to the ngrx discussion thread from a few months back:
> >
> >
> https://lists.apache.org/thread.html/06a59ea42e8d9a9dea5f90aab4011e44434555f8b7f3cf21297c7c87@%3Cdev.metron.apache.org%3E
> >
> > On Thu, May 2, 2019 at 1:17 PM Otto Fowler 
> > wrote:
> >
> > > If you can find a link in the archives for that thread, it would really
> > > help.
> > >
> > > I don’t think sending them up as one sensor would work…. as something
> > > quick. I think it is an interesting idea from a higher level that would
> > > need some more thought though ( IE: what if every sensor in the ui was
> a
> > > sensor group, and the existing entries where just groups of 1 ).
> > >
> > > As far as I can see, we have brought up the idea of a release
> ourselves,
> > I
> > > don’t see why we don’t just swarm this issue and get it right then
> > release.
> > >
> > >
> > >
> > > On May 2, 2019 at 04:16:31, Tamás Fodor (ftamas.m...@gmail.com) wrote:
> > >
> > > In PR#1360 we introduced a new state management strategy involving a
> new
> > > module called Ngrx. We had a discussion thread on this a few months ago
> > and
> > > we successfully convinced you about the benefits. This is one of the
> > > reasons why this PR is going to be still huge after cleaning up the
> > commit
> > > history. After you having a look at the changes and the feature itself,
> > > there's likely have questions about why certain parts work as they do.
> > The
> > > thing what I'd like to point out is that, yes, it probably takes more
> > time
> > > to get it in.
> > >
> > > In order to being able to release the RC, wouldn't it be an easy and
> > quick
> > > fix on the backend if it sent the aggregated parsers to the client as
> > they
> > > were one sensor? It's just an idea, it might be wrong, but at least we
> > > shouldn't have to wait until the aforementioned PR gets ready to be
> > merged
> > > to the master.
> > >
> > > On Wed, May 1, 2019 at 4:16 PM Justin Leet 
> > wrote:
> > >
> > > > Short version: I'm in favor of #2 of 0.7.1 and #1 as a blocker for
> > 0.8.0.
> > > > #3 seems like a total waste of time and effort.
> > > >
> > > > The wall of text version:
> > > > I agree this isn't "just the wrong thing shown", but for completely
> > > > different reasons.
> > > >
> > > > To be extremely clear about what the problem is: Our "dev"
> environment
> > > > (whose very name implies the audience is develops) uses a
> > > performance-based
> > > > advanced feature to ensure that all our of sample flows are regularly
> > run
> > > > and produce data. This feature has a bare minimal implementation to
> be
> > > > enabled via Ambari, which it currently is by default. This is because
> > of
> > > > the limited resources available that previously resulted in us
> turning
> > > off
> > > > Yaf, and therefore testing it during regular full dev runs. Right now
> > > > however, this feature is not exposed through the management UI, and
> > > > therefore it isn't obvious what the implications are. Am I missing
> > > anything
> > > > here?
> > > >
> > > > For users actually choosing to use the parser aggregation feature in
> a
> > > > non-full-dev environment, I'd expect substantially more care to be

Re: [DISCUSS] Add ngrx to handle state management in Angular

[Tibor Meller]

NgRx also makes testing easier and the architecture more straightforward.
Components in a Redux/NgRx architecture only responsible for rendering and
dispatching events.
All the data alternation implemented in pure functions so-called effects
and reducers.
No intertwined components, side effects just easily testable pure
functions.

+1 (non-binding)

On Tue, Nov 27, 2018 at 5:09 PM Shane Ardell 
wrote:

> There are a couple of good examples in Metron Alerts showing different
> advantages we would gain using NgRx. First, you’ll notice that a lot of
> state does not persist between view changes. For example, if a user inputs
> search criteria into the search bar, switches to the PCAP view, then
> switches back, that search input is not persisted. If we used NgRx, the
> application store can persist this state between view changes if we wanted
> to. The importance of this will continue to become more apparent as the
> application scales up and different views are added.
>
> In addition, there is quite a bit of shared application data between
> components. For example, you’ll see that both the GroupByComponent and
> AlertFiltersComponent display the number of facets available. When you
> filter by a facet, these numbers update. You can filter by clicking on the
> facets available in the AlertFiltersComponent, or you can search with a
> filter keyword and value in the search bar. Multiple areas of the
> application are showing the same data, and multiple areas of the
> application are manipulating that data. While passing data through @Input
> and @Output decorators and services is fine for basic sharing of
> application data, you’ll eventually create a complex web of data sharing
> and mutation that can be hard to follow and debug. In my opinion, we are
> nearing that point in our application. NgRx solves this with its
> one-direction data flow and immutable data structures. The store is the
> single source of truth which your component derives state from rather than
> having components holding their own state and having to manage, communicate
> and pass data between them.
>
> There are also many debugging advantages. I don’t have a specific example
> at the moment, but you can probably imagine how much easier it is to “time
> travel” with NgRx debugging tools. What I mean by “time travel” is the
> ability to move back and forth among the previous application states and
> view the results in real-time rather than setting a breakpoint, reloading
> the application, inspecting, setting another breakpoint to inspect a state
> previous to your current breakpoint, reloading the application, etc. This
> is a very repetitive and time-consuming thing to do when debugging on the
> front-end, and is very common. For a visual example of this, here is a
> video containing a decent overview of debugging with NgRx:
> https://www.youtube.com/watch?v=70ojPxMA7Ig
>
> On Tue, Nov 27, 2018 at 5:30 AM James Sirota  wrote:
>
> > I found a helpful article here:
> > https://brianflove.com/2018/01/08/ngrx-the-basics/
> >
> > A lot of this goes over my head, but in a nutshell, it's a tree-based
> > state management object for JS.  Its main drawback seems to be added
> > complexity, but if the guys who are more familiar with UI say we would
> > benefit from it I am inclined to take them at their word.
> >
> > 26.11.2018, 13:09, "Michael Miklavcic" :
> > > Shane, thanks for sharing this. Can you perhaps describe a sample use
> > case
> > > in the UI currently and explain for us how it currently works (or
> > doesn't,
> > > ha) versus how it would be modified and improved with using NgRx?
> > >
> > > Thanks,
> > > Mike
> > >
> > > On Fri, Nov 23, 2018 at 7:44 AM Shane Ardell  >
> > > wrote:
> > >
> > >>  What I'm referring to is roughly the entire contents of the UI
> > >>  application's memory.
> > >>
> > >>  On Thu, Nov 22, 2018 at 6:29 PM Otto Fowler  >
> > >>  wrote:
> > >>
> > >>  > Can you describe what you mean by “state” in a little more detail?
> > Not a
> > >>  > complete description, maybe just a crib list.
> > >>  >
> > >>  >
> > >>  > On November 22, 2018 at 07:21:43, Shane Ardell (
> > shane.m.ard...@gmail.com
> > >>  )
> > >>  > wrote:
> > >>  >
> > >>  > As both the Management and Alerts UI grow in size, managing
> > application
> > >>  > state continues to become more and more complex. To help us deal
> with
> > >>  > managing all of this state and ensuring our application derives
> state
> > >>  from
> > >>  > a single source of truth, I suggest we start using NgRx, a state
> > >>  > management
> > >>  > library based on the Redux pattern but built for Angular. It is by
> > far
> > >>  the
> > >>  > most popular library of this type for Angular. As you can see in
> the
> > >>  > project's GitHub Insights tab <
> > https://github.com/ngrx/platform/pulse>,
> > >>  > it's quite actively worked on and releases are pretty frequent. The
> > >>  > project
> > >>  > is licensed under MIT.
> > >>  >
> > >>  > As far as an 

Re: [DISCUSS] Add ngrx to handle state management in Angular

[Tibor Meller]

Shane, Thanks for gathering the information and raising this.
I also feel that our UIs reached a level of complexity what makes this to a
reasonable next step.
This complexity on the client side will grow in the future and I believe it
is better to prepare instead of trying to make huge refactorings later.
NgRx/Redux is quiet the industry standard architectural library in both
React and Angular realm. Pretty well known in the community.
I would be happy to move forward with NgRx.

On Fri, Nov 23, 2018 at 3:44 PM Shane Ardell 
wrote:

> What I'm referring to is roughly the entire contents of the UI
> application's memory.
>
> On Thu, Nov 22, 2018 at 6:29 PM Otto Fowler 
> wrote:
>
> > Can you describe what you mean by “state” in a little more detail?  Not a
> > complete description, maybe just a crib list.
> >
> >
> > On November 22, 2018 at 07:21:43, Shane Ardell (shane.m.ard...@gmail.com
> )
> > wrote:
> >
> > As both the Management and Alerts UI grow in size, managing application
> > state continues to become more and more complex. To help us deal with
> > managing all of this state and ensuring our application derives state
> from
> > a single source of truth, I suggest we start using NgRx, a state
> > management
> > library based on the Redux pattern but built for Angular. It is by far
> the
> > most popular library of this type for Angular. As you can see in the
> > project's GitHub Insights tab ,
> > it's quite actively worked on and releases are pretty frequent. The
> > project
> > is licensed under MIT.
> >
> > As far as an approach to integration, I don't think we necessarily need a
> > big refactoring right off the bat. I feel something like this can be done
> > in a piecemeal approach over time. I think we can start by introducing it
> > into the project the next time we have a new application feature.
> >
> > What are everyone's thoughts around this?
> >
> > Cheers,
> > Shane
> >
> >
>

Re: [DISCUSS] Migrate from Protractor to Cypress

[Tibor Meller]

Hi All,

To continue this thread and keep you updated I would like to share that I'm
opened a PR with Cypress UI tests for PCAP.
I was also able to integrate these tests into Travis CI.
https://github.com/apache/metron/pull/1226
You can find more information in the PR description.

I also opened a new Jira ticket to track a possible migration to Cypress
from Protractor.
https://issues.apache.org/jira/browse/METRON-1848
In this ticket, I collected the existing UI tests written in protractor.
Also added links to this thread and the previous related tickets to make it
easier to follow this discussion.

Please let me know what do you think about a migration to Cypress as a next
step!

Thanks,
Tibor

On Thu, Sep 27, 2018 at 9:23 AM Tibor Meller  wrote:

> Great Guys! Thanks for the feedback. I'll move forward as discussed.
>
> Thx
>
> On Wed, Sep 26, 2018 at 11:44 PM Michael Miklavcic <
> michael.miklav...@gmail.com> wrote:
>
>> I'm good with it. We can see some tests in action (and hopefully running
>> in
>> Travis! :-D) and then migrate and deprecate Protractor accordingly if we
>> still agree that's the way to go. When you submit the first PR, please
>> link
>> to this DISCUSS via permalink from the mailing list archives. Thanks guys.
>>
>> Cheers,
>> Mike
>>
>> On Wed, Sep 26, 2018 at 7:17 AM Shane Ardell 
>> wrote:
>>
>> > I think Tibor's idea of using PCAP tests as an introduction to Cypress
>> for
>> > Metron is a great idea. As he pointed out, PCAP tests can take
>> advantage of
>> > Cypress' capability to mock responses, and we can set it up to run in
>> > Travis. Once the community is able to see the benefits from an actual
>> set
>> > of Cypress tests inside the project and running in Travis, I think any
>> > questions about migrating the rest of the existing tests from
>> Protractor to
>> > Cypress will be settled. However, if for some reason we run into issues
>> > implementing or running the tests, we will have invested a fraction of
>> time
>> > vs. migrating all the tests right away.
>> >
>> > On Wed, Sep 26, 2018 at 2:12 PM Tibor Meller 
>> > wrote:
>> >
>> > > Hi Team,
>> > >
>> > > Many of us agreed on that Cypress could be a more capable tool for us
>> to
>> > > write high-level UI tests, whether those be e2e, integration or
>> automated
>> > > regression tests. If there is no open question left about cypress we
>> > could
>> > > to bring it a test drive. My suggestion is to implement the PCAP UI
>> tests
>> > > with Cypress. Some services and PCAP semple data yet not available
>> from
>> > our
>> > > CI environment so protractor is hardly applicable here. This would be
>> a
>> > > great opportunity for cypress to shine. With Cypress, we are able to
>> mock
>> > > out those responses and make it run in Travis.
>> > > Anytime we make PCAP data available in Travis we could be able to plug
>> > out
>> > > those mocks and run the same test as integration or e2e tests if we
>> like.
>> > >
>> > > Because it is relatively easy to migrate across cypress and
>> protractor I
>> > > see no major risks here if we decide to stick with Protractor for some
>> > > reason.
>> > >
>> > > What do you think?
>> > >
>> > > Thanks for your feedback,
>> > > Tibor
>> > >
>> > > On Wed, Sep 19, 2018 at 1:49 PM Shane Ardell <
>> shane.m.ard...@gmail.com>
>> > > wrote:
>> > >
>> > > > Hello everyone,
>> > > >
>> > > > Currently, we use Protractor to run our UI "end-to-end" tests.
>> However,
>> > > > there are a handful of major advantages we can gain from switching
>> to
>> > > > Cypress: https://www.cypress.io/features/.
>> > > >
>> > > >- As with most Selenium-based e2e testing frameworks, Protractor
>> > > suffers
>> > > >from test flakiness. This is because Selenium runs outside of the
>> > > > browser
>> > > >and executes remote commands across the network. To work around
>> this
>> > > at
>> > > > the
>> > > >moment, we are using protractor-flake to re-run failed tests, but
>> > this
>> > > > is
>> > > >more of a crutch than a fix. Cypress executes in the same run
>> loop
>> > as
>> > > > the
>> > > >application it's testing, and as a result does not suffer from
>> the
>> > > same
>> > > >flakiness.
>> > > >- As a result of its architecture, Cypress runs much faster than
>> > > >Protractor. This is especially critical if e2e tests are added to
>> > the
>> > > CI
>> > > >build in the future.
>> > > >- Protractor is incredibly hard to debug. In contrast, Cypress
>> comes
>> > > >with a plethora of debugging features, some of which you can see
>> in
>> > > > action
>> > > >here: https://vimeo.com/242961930#t=264s
>> > > >
>> > > > Does anyone else have thoughts or opinions on switching to Cypress
>> or
>> > > > staying with Protractor?
>> > > >
>> > > > Cheers,
>> > > > Shane
>> > > >
>> > >
>> >
>>
>

Re: Invite to Slack Channel

[Tibor Meller]

Hi Guys,
Can you add me to the apache metron slack chanel?

Thanks,

On Thu, Oct 4, 2018 at 1:14 PM Otto Fowler  wrote:

> Done
>
>
> On October 4, 2018 at 05:35:06, Tamás Fodor (ftamas.m...@gmail.com) wrote:
>
> Hello,
>
> Michael, can you add me as well?
>
> Thank you in advance!
>
> Tamas
>
> On Wed, Oct 3, 2018 at 4:27 PM Michael Miklavcic <
> michael.miklav...@gmail.com> wrote:
>
> > Sent
> >
> > On Wed, Oct 3, 2018 at 8:17 AM Shane Ardell 
> > wrote:
> >
> > > Hello everyone,
> > >
> > > Is it possible for someone to send me an invite to the Metron Slack
> > > channel?
> > >
> > > Regards,
> > > Shane
> > >
> >
>

Re: [DISCUSS] Switching to a better alternative of Pikaday.js

[Tibor Meller]

Thanks, Tamas! The plan you described seems a good approach to me.
Let's wait another day before we create Jira tickets from these steps to
make sure no one else has other concerns.

One more question: how much of these changes could be applicable to the
Management UI?
It would be great plus to see those two UI getting closer to each other
with the underlying technologies instead of shifting away.

On Tue, Oct 16, 2018 at 9:37 AM Tamás Fodor  wrote:

> I'm agreeing with moving forward with Ng Bootstrap. In order to do that, I
> think it would be a good start to refactor those components which use the
> obsolete jquery based vesrion of bootstrap. Shane has already started it by
> refactoring the Modal dialog in the management UI (We also have this
> component in the alerts UI). Once we've succeeded that, we can remove
> (hopefully) jQuery form the dependency list.
> As the second step, we should migrate to Ng bootstrap. I'm assuming that if
> the jquery based Boostrap is replaced with Ng bootstrap, we're still good
> since we're using only classes from the bootstrap CSS which is shared
> across jQuery bootstrap and Ng bootstrap.
> If everything is good and nothing is broken we can start working on the new
> date/time picker component based on Ng bootstrap and then we can get rid of
> Pikaday.
> Once Pikaday is completely eliminated from the system, we can be sure that
> the PR about eliminating moment.js is ready to go.
>
> Cheers,
> Tamas
>
> On Mon, Oct 15, 2018 at 8:36 PM Michael Miklavcic <
> michael.miklav...@gmail.com> wrote:
>
> > I like point #3 from Tibor - you can pick and choose how you compose the
> > date/time pickers. It would be nice to not have times required as a
> > dropdown at some point or another, or at the very least something we can
> > customize differently to our liking :)
> >
> > Per the comments from Tamas, Shane, and Tibor, is there any reason we
> > wouldn't want to move forward with the Angular port of Bootstrap, NG
> > Bootstrap? Per your arguments for it, this sounds like the right path
> > forward to me. I'm +1 on this approach provided someone doesn't come back
> > with a "well, there's only this problem..."
> >
> > Best,
> > Mike
> >
> > On Mon, Oct 15, 2018 at 7:39 AM Shane Ardell 
> > wrote:
> >
> > > We are definitely on the same page.
> > >
> > > On Mon, Oct 15, 2018 at 2:06 PM Tibor Meller 
> > > wrote:
> > >
> > > > @Shane It seems like we're agreed on this. :D
> > > >
> > > > On Mon, Oct 15, 2018 at 1:18 PM Tibor Meller  >
> > > > wrote:
> > > >
> > > > > Hi Guys,
> > > > >
> > > > > I think we could consider moving to ng bootstrap. It solves most of
> > our
> > > > > problems and makes our code base cleaner easier to maintain.
> > > > >
> > > > > Here are some benefits I see:
> > > > > 1. we could eliminate jQuery from the code base
> > > > > Currently, we're using bootstrap but an implementation based on
> > jQuery.
> > > > > Angular and jQuery doesn't build to live together.
> > > > > 2. NgBootstrap made to being used with Angular
> > > > > It uses Angular instead of hacking the rendering/dom manipulation
> > with
> > > > > jQuery.
> > > > > 3. It contains a date and a time picker.
> > > > > It's easy to combine to a datetime picker.
> > > > > 4. No dependencies.
> > > > > By changing currently used bootstrap to NgBootstrap we could clear
> > > > jquery,
> > > > > moment, pickaday, pickaday-time libraries from our dependencies.
> > > > >
> > > > > Another huge advantage of NgBootstrap is that we don't have to
> > rewrite
> > > > > anything we don't want to. Our UI already uses Bootstrap.
> > > > >
> > > > > What do you guys think?
> > > > >
> > > > > On Wed, Oct 10, 2018 at 4:19 PM Nick Allen 
> > wrote:
> > > > >
> > > > >> > Before making a decision on what's next, I'd to ask you a
> > question.
> > > Is
> > > > >> it really
> > > > >> a priority and is it really worth the effort to touch our
> currently
> > > used
> > > > >> date picker component to get ~15% reduction in the bundle size by
> > > > removing
> > > > >> moment?
> > > > >>
> > > > >> As an aside, I think there i

Re: [DISCUSS] Switching to a better alternative of Pikaday.js

[Tibor Meller]

@Shane It seems like we're agreed on this. :D

On Mon, Oct 15, 2018 at 1:18 PM Tibor Meller  wrote:

> Hi Guys,
>
> I think we could consider moving to ng bootstrap. It solves most of our
> problems and makes our code base cleaner easier to maintain.
>
> Here are some benefits I see:
> 1. we could eliminate jQuery from the code base
> Currently, we're using bootstrap but an implementation based on jQuery.
> Angular and jQuery doesn't build to live together.
> 2. NgBootstrap made to being used with Angular
> It uses Angular instead of hacking the rendering/dom manipulation with
> jQuery.
> 3. It contains a date and a time picker.
> It's easy to combine to a datetime picker.
> 4. No dependencies.
> By changing currently used bootstrap to NgBootstrap we could clear jquery,
> moment, pickaday, pickaday-time libraries from our dependencies.
>
> Another huge advantage of NgBootstrap is that we don't have to rewrite
> anything we don't want to. Our UI already uses Bootstrap.
>
> What do you guys think?
>
> On Wed, Oct 10, 2018 at 4:19 PM Nick Allen  wrote:
>
>> > Before making a decision on what's next, I'd to ask you a question. Is
>> it really
>> a priority and is it really worth the effort to touch our currently used
>> date picker component to get ~15% reduction in the bundle size by removing
>> moment?
>>
>> As an aside, I think there is a greater benefit here too.  We need to make
>> a conscious effort to identify libraries that we are using that are
>> deprecated, lack community support, and are unlikely to be maintained and
>> updated for security vulnerabilities.  We need to actively identify and
>> replace those.
>>
>>
>>
>>
>>
>> On Wed, Oct 10, 2018 at 9:33 AM Tamás Fodor 
>> wrote:
>>
>> > I'd like to open a discussion about switching to a new date picker
>> library
>> > in the Metron Alerts UI regarding to the following:
>> >
>> >
>> >
>> https://lists.apache.org/thread.html/2e4fafa4256ce14ebcd4433420974e24962884204418ade51f0e3bfb@%3Cdev.metron.apache.org%3E
>> >
>> > https://github.com/apache/metron/pull/1219#discussion_r223733562
>> >
>> > A week ago, I opened a PR about removing moment.js from the code base to
>> > decrease the size of the production javascript bundle. I could achieve
>> 15%
>> > loss in the final bundle size which is admittedly not a game changer but
>> > still. Not to mention if we want to heavily rely on date manipulator
>> > functions in the future it's better to get rid of it at this early
>> stage.
>> > Go here <https://github.com/apache/metron/pull/1219> to read more about
>> > the
>> > background and the results. I tried to provide as many details as I
>> could.
>> >
>> > So far, so good. But then I stumbled upon an obstacle, Pikaday
>> > <https://github.com/dbushell/Pikaday>.
>> >
>> > Before going further, let me thank Tibor Meller, Michael Miklavcic and
>> > Nicholas Allen for taking their time to go through my proposal to deal
>> with
>> > the aforementioned issue. At the end, we agreed on basically not going
>> with
>> > my temporary solution that intended to solve the related problems of
>> > Pikaday and we'd rather like to find and change for a better
>> alternative.
>> >
>> > To be fair, Pikaday is a pretty good date picker module, its only
>> problem
>> > is the moment dependency if it's installed via npm. But other than
>> that, it
>> > functions perfectly. Zero dependencies, small, etc. Long story short,
>> it's
>> > good for us unless we want to get rid of moment.js.
>> >
>> > Before making a decision on what's next, I'd to ask you a question. Is
>> it
>> > really a priority and is it really worth the effort to touch our
>> currently
>> > used date picker component to get ~15% reduction in the bundle size by
>> > removing moment?
>> >
>> > I'm asking it because if we want to do so, considering that it's a huge
>> > topic, the following questions might come up:
>> >
>> > *A: What component do we want to use instead of Pikaday?*
>> >
>> > I'm not satisfied with the alternative individual solutions out there on
>> > npm. I'd rather pick a component library like the angular port of
>> Bootstrap
>> > <https://ng-bootstrap.github.io/#/home> or the angular material library
>> > <https://material.angular.io/>. Both of them have a date picker
>> component
>> > a

Re: [DISCUSS] Switching to a better alternative of Pikaday.js

[Tibor Meller]

Hi Guys,

I think we could consider moving to ng bootstrap. It solves most of our
problems and makes our code base cleaner easier to maintain.

Here are some benefits I see:
1. we could eliminate jQuery from the code base
Currently, we're using bootstrap but an implementation based on jQuery.
Angular and jQuery doesn't build to live together.
2. NgBootstrap made to being used with Angular
It uses Angular instead of hacking the rendering/dom manipulation with
jQuery.
3. It contains a date and a time picker.
It's easy to combine to a datetime picker.
4. No dependencies.
By changing currently used bootstrap to NgBootstrap we could clear jquery,
moment, pickaday, pickaday-time libraries from our dependencies.

Another huge advantage of NgBootstrap is that we don't have to rewrite
anything we don't want to. Our UI already uses Bootstrap.

What do you guys think?

On Wed, Oct 10, 2018 at 4:19 PM Nick Allen  wrote:

> > Before making a decision on what's next, I'd to ask you a question. Is
> it really
> a priority and is it really worth the effort to touch our currently used
> date picker component to get ~15% reduction in the bundle size by removing
> moment?
>
> As an aside, I think there is a greater benefit here too.  We need to make
> a conscious effort to identify libraries that we are using that are
> deprecated, lack community support, and are unlikely to be maintained and
> updated for security vulnerabilities.  We need to actively identify and
> replace those.
>
>
>
>
>
> On Wed, Oct 10, 2018 at 9:33 AM Tamás Fodor  wrote:
>
> > I'd like to open a discussion about switching to a new date picker
> library
> > in the Metron Alerts UI regarding to the following:
> >
> >
> >
> https://lists.apache.org/thread.html/2e4fafa4256ce14ebcd4433420974e24962884204418ade51f0e3bfb@%3Cdev.metron.apache.org%3E
> >
> > https://github.com/apache/metron/pull/1219#discussion_r223733562
> >
> > A week ago, I opened a PR about removing moment.js from the code base to
> > decrease the size of the production javascript bundle. I could achieve
> 15%
> > loss in the final bundle size which is admittedly not a game changer but
> > still. Not to mention if we want to heavily rely on date manipulator
> > functions in the future it's better to get rid of it at this early stage.
> > Go here <https://github.com/apache/metron/pull/1219> to read more about
> > the
> > background and the results. I tried to provide as many details as I
> could.
> >
> > So far, so good. But then I stumbled upon an obstacle, Pikaday
> > <https://github.com/dbushell/Pikaday>.
> >
> > Before going further, let me thank Tibor Meller, Michael Miklavcic and
> > Nicholas Allen for taking their time to go through my proposal to deal
> with
> > the aforementioned issue. At the end, we agreed on basically not going
> with
> > my temporary solution that intended to solve the related problems of
> > Pikaday and we'd rather like to find and change for a better alternative.
> >
> > To be fair, Pikaday is a pretty good date picker module, its only problem
> > is the moment dependency if it's installed via npm. But other than that,
> it
> > functions perfectly. Zero dependencies, small, etc. Long story short,
> it's
> > good for us unless we want to get rid of moment.js.
> >
> > Before making a decision on what's next, I'd to ask you a question. Is it
> > really a priority and is it really worth the effort to touch our
> currently
> > used date picker component to get ~15% reduction in the bundle size by
> > removing moment?
> >
> > I'm asking it because if we want to do so, considering that it's a huge
> > topic, the following questions might come up:
> >
> > *A: What component do we want to use instead of Pikaday?*
> >
> > I'm not satisfied with the alternative individual solutions out there on
> > npm. I'd rather pick a component library like the angular port of
> Bootstrap
> > <https://ng-bootstrap.github.io/#/home> or the angular material library
> > <https://material.angular.io/>. Both of them have a date picker
> component
> > and many other components to rely on and reuse throughout the Metron app.
> >
> > *B: What component library do we want to use?*
> >
> > Introducing a new component library requires a lot of research and there
> > are many things we have to agreed on. Since it's a long term plan because
> > it would be great to use it consistently instead of picking a new one a
> few
> > months later just because we chose wrongly.
> >
> > *C: What about the jQuery version of Bootstrap?*
> >
> > So bas

Re: [DISCUSS] Migrate from Protractor to Cypress

[Tibor Meller]

Great Guys! Thanks for the feedback. I'll move forward as discussed.

Thx

On Wed, Sep 26, 2018 at 11:44 PM Michael Miklavcic <
michael.miklav...@gmail.com> wrote:

> I'm good with it. We can see some tests in action (and hopefully running in
> Travis! :-D) and then migrate and deprecate Protractor accordingly if we
> still agree that's the way to go. When you submit the first PR, please link
> to this DISCUSS via permalink from the mailing list archives. Thanks guys.
>
> Cheers,
> Mike
>
> On Wed, Sep 26, 2018 at 7:17 AM Shane Ardell 
> wrote:
>
> > I think Tibor's idea of using PCAP tests as an introduction to Cypress
> for
> > Metron is a great idea. As he pointed out, PCAP tests can take advantage
> of
> > Cypress' capability to mock responses, and we can set it up to run in
> > Travis. Once the community is able to see the benefits from an actual set
> > of Cypress tests inside the project and running in Travis, I think any
> > questions about migrating the rest of the existing tests from Protractor
> to
> > Cypress will be settled. However, if for some reason we run into issues
> > implementing or running the tests, we will have invested a fraction of
> time
> > vs. migrating all the tests right away.
> >
> > On Wed, Sep 26, 2018 at 2:12 PM Tibor Meller 
> > wrote:
> >
> > > Hi Team,
> > >
> > > Many of us agreed on that Cypress could be a more capable tool for us
> to
> > > write high-level UI tests, whether those be e2e, integration or
> automated
> > > regression tests. If there is no open question left about cypress we
> > could
> > > to bring it a test drive. My suggestion is to implement the PCAP UI
> tests
> > > with Cypress. Some services and PCAP semple data yet not available from
> > our
> > > CI environment so protractor is hardly applicable here. This would be a
> > > great opportunity for cypress to shine. With Cypress, we are able to
> mock
> > > out those responses and make it run in Travis.
> > > Anytime we make PCAP data available in Travis we could be able to plug
> > out
> > > those mocks and run the same test as integration or e2e tests if we
> like.
> > >
> > > Because it is relatively easy to migrate across cypress and protractor
> I
> > > see no major risks here if we decide to stick with Protractor for some
> > > reason.
> > >
> > > What do you think?
> > >
> > > Thanks for your feedback,
> > > Tibor
> > >
> > > On Wed, Sep 19, 2018 at 1:49 PM Shane Ardell  >
> > > wrote:
> > >
> > > > Hello everyone,
> > > >
> > > > Currently, we use Protractor to run our UI "end-to-end" tests.
> However,
> > > > there are a handful of major advantages we can gain from switching to
> > > > Cypress: https://www.cypress.io/features/.
> > > >
> > > >- As with most Selenium-based e2e testing frameworks, Protractor
> > > suffers
> > > >from test flakiness. This is because Selenium runs outside of the
> > > > browser
> > > >and executes remote commands across the network. To work around
> this
> > > at
> > > > the
> > > >moment, we are using protractor-flake to re-run failed tests, but
> > this
> > > > is
> > > >more of a crutch than a fix. Cypress executes in the same run loop
> > as
> > > > the
> > > >application it's testing, and as a result does not suffer from the
> > > same
> > > >flakiness.
> > > >- As a result of its architecture, Cypress runs much faster than
> > > >Protractor. This is especially critical if e2e tests are added to
> > the
> > > CI
> > > >build in the future.
> > > >- Protractor is incredibly hard to debug. In contrast, Cypress
> comes
> > > >with a plethora of debugging features, some of which you can see
> in
> > > > action
> > > >here: https://vimeo.com/242961930#t=264s
> > > >
> > > > Does anyone else have thoughts or opinions on switching to Cypress or
> > > > staying with Protractor?
> > > >
> > > > Cheers,
> > > > Shane
> > > >
> > >
> >
>

Re: [DISCUSS] Migrate from Protractor to Cypress

[Tibor Meller]

Hi Team,

Many of us agreed on that Cypress could be a more capable tool for us to
write high-level UI tests, whether those be e2e, integration or automated
regression tests. If there is no open question left about cypress we could
to bring it a test drive. My suggestion is to implement the PCAP UI tests
with Cypress. Some services and PCAP semple data yet not available from our
CI environment so protractor is hardly applicable here. This would be a
great opportunity for cypress to shine. With Cypress, we are able to mock
out those responses and make it run in Travis.
Anytime we make PCAP data available in Travis we could be able to plug out
those mocks and run the same test as integration or e2e tests if we like.

Because it is relatively easy to migrate across cypress and protractor I
see no major risks here if we decide to stick with Protractor for some
reason.

What do you think?

Thanks for your feedback,
Tibor

On Wed, Sep 19, 2018 at 1:49 PM Shane Ardell 
wrote:

> Hello everyone,
>
> Currently, we use Protractor to run our UI "end-to-end" tests. However,
> there are a handful of major advantages we can gain from switching to
> Cypress: https://www.cypress.io/features/.
>
>- As with most Selenium-based e2e testing frameworks, Protractor suffers
>from test flakiness. This is because Selenium runs outside of the
> browser
>and executes remote commands across the network. To work around this at
> the
>moment, we are using protractor-flake to re-run failed tests, but this
> is
>more of a crutch than a fix. Cypress executes in the same run loop as
> the
>application it's testing, and as a result does not suffer from the same
>flakiness.
>- As a result of its architecture, Cypress runs much faster than
>Protractor. This is especially critical if e2e tests are added to the CI
>build in the future.
>- Protractor is incredibly hard to debug. In contrast, Cypress comes
>with a plethora of debugging features, some of which you can see in
> action
>here: https://vimeo.com/242961930#t=264s
>
> Does anyone else have thoughts or opinions on switching to Cypress or
> staying with Protractor?
>
> Cheers,
> Shane
>

[DISCUSS] PCAP data for testing and development

[Tibor Meller]

Hi all,

I would like to start a discussion on the possible ways to provide PCAP
data for the full dev.
The full dev VM after a rebuild contains no PCAP data. Currently,
I'm uploading binaries manually. This makes development slower and testing
problematic as well. I think a more desired outcome would be
something similar to what we have in the Alert tab, which is to have some
pcap data available right after starting the VM.

Do you guys think uploading pcap sample date as part of the
ansible provisioning step would be a good approach?
Or sensor stubs for pcap would be a better way?

I would be curious about your thoughts!

Thanks,
Tibor

Metron-alerts UI Unit Tests

[Tibor Meller]

Hi,

I'm writing to inform the team about the current state of unit tests in the
angular client and start a discussion about it in general.
Probably some of you know the in the previous weeks we fixed the existing
errors in all the *spec.ts files in /metron-interface/metron-alerts which
makes as able to run and extend our unit tests on the client side code.

In its current state, most of the existing spec/test files just
instantiating some of our components and make assertation to the
existence of them. It might sound bad having that in the current state of
the project, but it's never too late to start to improve things. :)

So what I like to ask from the team is if you making changes in the alert
UI make sure you run one of the following commands check unit tests:
*ng test*
*npm test*

If you're curious about the coverage you can use:
*ng test --code-coverage*
to get a short summary at the end of the process. A very detailed coverage
report also generates to coverage folder in this case.

*Please make sure your changes covered by unit tests and not decreasing the
coverage.*
We are also planning to make UI unit tests part of our travis build process.
With that and some extra care, I'm pretty sure that we can build up a solid
base of coverage and prevent many of bugs in the future.

If you have any suggestion on how to improve or opinions on the topic
please let me know.

Regards,
Tibor

Metron-alerts E2E testing

[Tibor Meller]

Hi All,

Probably some of you already know that Shane and I working on stabilizing
and extending e2e tests in metron-interface/metron-alerts.
Currently, the tests using Protractor which is the default e2e testing
framework of Angular.
The tests injecting data right into ElasticSearch and making assertation on
the alerts UI. This means the tests only end to end from a UI perspective.
However, even with that, they are very useful in the validation of UI and
to prevent regressions.

It the current state of the development it would be crucial to these tests
being run by every developer who making changes on the alerts UI code or in
the Metron REST service.
Without that, it is very hard to tell whether the tests are just flaky or
the changes brake some of them.

So please start to experimentaling with this tool, and let us know how it
works out for you.

You can find more information about setup and run on this link:
https://github.com/tiborm/metron/blob/master/metron-interface/metron-alerts/README.md#e2e-tests

Also if you have any opinion or idea on how to improve just let us know.

Cheers,
Tibor