Github user tiborm commented on the issue:
https://github.com/apache/metron/pull/1103
Thanks to everyone for the feedback and comments!
---
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1103
This has been merged into the feature branch. Can you close it @tiborm?
---
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/1103
Looks like all comments have been addressed, reviewers have been able to
test successfully, and any remaining follow-on work has been added to the
feature branch Jira/Epic. I'm +1 by inspection. Ni
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1103
@sardell Thanks for adding the tickets.
+1, @tiborm Thanks for your patience during the whole review process. I'm
looking forward to getting the ball rolling on the rest of the contribut
Github user sardell commented on the issue:
https://github.com/apache/metron/pull/1103
@justinleet Here are the tickets for the follow-on items:
* Input validation: https://issues.apache.org/jira/browse/METRON-1712
* Kill button for in progress jobs:
https://issues.apache.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1103
@sardell I'm fine with this stuff being follow on. Can we make sure we
have tickets linked to https://issues.apache.org/jira/browse/METRON-1554?
I believe the follow-ons are
-
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1103
Several people have given feedback on this PR. What do you think
@mmiklavc, @justinleet and @cestella? Is this good enough for a first pass at
the UI?
---
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1103
Given that unit tests will be addressed in a forthcoming PR, this is good
enough for me as an initial UI. +1 pending other commenters are satisfied.
---
Github user sardell commented on the issue:
https://github.com/apache/metron/pull/1103
@justinleet In response to your UI-related questions:
> Better input validation would be nice. I can currently enter negative
ports (which don't match anything) or ports beyond max int (whic
Github user tiborm commented on the issue:
https://github.com/apache/metron/pull/1103
As part of the latest commits I removed commented code blocks, and fixed
the variable naming issues in pcap-packet-line.component.ts.
This PR and the followup ones are updated by the latest change
Github user tiborm commented on the issue:
https://github.com/apache/metron/pull/1103
@mmiklavc Thanks for the comment! I extended the JIRA ticket with user
story like test scenarios.
Also added a short description of to the PR description about how to spin
up a full dev with pcap
Github user tiborm commented on the issue:
https://github.com/apache/metron/pull/1103
@mmiklavc The list of the sequential PR's in order are the following:
METRON-1671: Initial PCAP UI
https://github.com/apache/metron/pull/1103
METRON-1662: Adding download button
Github user tiborm commented on the issue:
https://github.com/apache/metron/pull/1103
@cestella We added the license headers for all new files.
---
Github user tiborm commented on the issue:
https://github.com/apache/metron/pull/1103
@mmiklavc We had to update a package-lock.json file because of the original
one contained a package collision. npm ci command just failed on that.
---
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1103
In the screenshots you provide, the timestamp field contains both the epoch
timestamp and the date. I'd have to dig in, but based on PDML files I've seen,
it's displaying both the "show" and the
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/1103
@tiborm - I see a lot of recent changes to dependency versions in the
package lock file - is that intentional? Just a short comment about what it's
for would be helpful.
---
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1103
@mmiklavc The job manager is asynchronous in that it accepts a Finalizer.
We have the polling loop in place but we would need to refactor the job manager
to expose a callback function for getStat
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/1103
> I'm not sure the Hadoop Job object is asynchronous anyways.
@merrimanr If I'm right on the context of the Q and A here, the PcapJob
around the underlying MR job handles it async or sync. For
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1103
@merrimanr Up front I want to say I agree async is follow-on. I think
polling is fine for right now, but there are potential scaling concerns and I
wanted to get a sense of the reasoning. In pr
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1103
@justinleet I disagree with "Polling seems pretty avoidable". I feel like
that would be significant work and I'm not sure the Hadoop Job object is
asynchronous anyways. We would have to poll som
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1103
The other feedback I have is that the unit tests need some work. For
example, when I updated `pcap.request.ts` but hadn't updated
`pcap-filters.component.html`, the unit tests still pass. That t
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1103
Can I also get some insight into why the job status REST call and UI usage
isn't an asynchronous call? Polling seems pretty avoidable here.
---
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1103
I have a couple comments after spinning up #1122 (which should have all
these changes, I believe).
- Better input validation would be nice. I can currently enter negative
ports (which
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/1103
> Have you included steps to reproduce the behavior or problem that is
being changed or addressed?
>
> Have you included steps or a guide to how the change may be verified and
tested man
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1103
I tested this in full dev and the fixed query request is incorrect. Here's
what is being sent:
```
{
"startTime": 0,
"endTime": 15,
"srcIp": "",
"sr
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/1103
I have submitted a PR to up the rat plugin version and have fixed it in
master #1126, let's make sure to add license headers here for new files added
in this branch though.
---
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1103
There's a bug report that rat treats .ts files as binary and ignores them.
https://issues.apache.org/jira/browse/RAT-234
---
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/1103
@tiborm @sardell Can one of you list out the UI PR's and indicate the order
in which they should be tested and subsequently merged into the feature branch?
Since this is the initial major ticket, y
Github user james-sirota commented on the issue:
https://github.com/apache/metron/pull/1103
I reviewed the UI. Everything comes up fine and looks good. One
assumption I am making is that protocol is numeric because the protocol field
will not allow me to enter a character. +1
---
Github user tiborm commented on the issue:
https://github.com/apache/metron/pull/1103
@mmiklavc The 10k+ line spec file is a working unit test which contains
mock data as well.
Unfortunately, some other unit tests failing in this PR. We already to a
separate ticket for fixing and
30 matches
Mail list logo