[GitHub] metron issue #1294: METRON-1930: Update webpack-dev-server in Alerts UI
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1294 I went through the testing guide and it worked as expected. ð ---
[GitHub] metron issue #1219: METRON-1796: [UI] Migrate off moment.js
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1219 Since we are not there yet to merge it back, I'm closing this issue. I can use this code later when we've solved all the prerequisites in order to satisfy the need of eliminating moment.js from the system. ---
[GitHub] metron pull request #1219: METRON-1796: [UI] Migrate off moment.js
Github user ruffle1986 closed the pull request at: https://github.com/apache/metron/pull/1219 ---
[GitHub] metron pull request #1226: METRON-1803: Integrate Cypress with Travis
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1226#discussion_r225168900
--- Diff: metron-interface/metron-alerts/package.json ---
@@ -5,12 +5,14 @@
"angular-cli": {},
"scripts": {
"build": "./node_modules/@angular/cli/bin/ng build --prod",
-"start": "ng serve",
+"start": "ng serve --aot",
--- End diff --
why is `--aot` needed? It slows down the compiler in development mode.
I'm assuming you wanted to be consistent with the
[start-dev](https://github.com/apache/metron/blob/master/metron-interface/metron-alerts/scripts/start-dev.sh#L19)
script, right?
---
[GitHub] metron pull request #1208: METRON-1790: Unsubscribe from every observable in...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1208#discussion_r224797583
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts
---
@@ -81,26 +81,28 @@ export class PcapPanelComponent implements OnInit,
OnDestroy {
this.pdml = null;
this.progressWidth = 0;
this.errorMsg = null;
-this.submitSubscription =
this.pcapService.submitRequest(pcapRequest).subscribe((submitResponse:
PcapStatusResponse) => {
- let id = submitResponse.jobId;
- if (!id) {
-this.errorMsg = submitResponse.description;
-this.queryRunning = false;
- } else {
-this.startPolling(id);
+this.subscriptions['submitSubscription'] =
this.pcapService.submitRequest(pcapRequest).subscribe(
+ (submitResponse: PcapStatusResponse) => {
+let id = submitResponse.jobId;
+if (!id) {
+ this.errorMsg = submitResponse.description;
+ this.queryRunning = false;
+} else {
+ this.startPolling(id);
+}
+ }, (error: any) => {
+this.errorMsg = `Response message: ${error.message}. Something
went wrong with your query submission!`;
--- End diff --
Yes because it's already implemented. Every time the pcap panel is rendered
(`ngOnInit` ) we ask the server whether we have a running job in the
background. If so, we reattach the job on the UI and keep pooling the server
until the job gets done. But it has nothing to do with what we're doing here in
this PR.
---
[GitHub] metron pull request #1208: METRON-1790: Unsubscribe from every observable in...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1208#discussion_r224366897
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts
---
@@ -81,26 +81,28 @@ export class PcapPanelComponent implements OnInit,
OnDestroy {
this.pdml = null;
this.progressWidth = 0;
this.errorMsg = null;
-this.submitSubscription =
this.pcapService.submitRequest(pcapRequest).subscribe((submitResponse:
PcapStatusResponse) => {
- let id = submitResponse.jobId;
- if (!id) {
-this.errorMsg = submitResponse.description;
-this.queryRunning = false;
- } else {
-this.startPolling(id);
+this.subscriptions['submitSubscription'] =
this.pcapService.submitRequest(pcapRequest).subscribe(
+ (submitResponse: PcapStatusResponse) => {
+let id = submitResponse.jobId;
+if (!id) {
+ this.errorMsg = submitResponse.description;
+ this.queryRunning = false;
+} else {
+ this.startPolling(id);
+}
+ }, (error: any) => {
+this.errorMsg = `Response message: ${error.message}. Something
went wrong with your query submission!`;
--- End diff --
In case of `statusSubscription`, we have to unsubscribe because we want to
stop polling the server if an error occurs.
But the other observables are one-time observables which means once they
complete (or throw an error) we won't get any additional event until we
subscribe again. So in theory, if would be very precise, we unsubscribed from
the observables in case of fulfilling not just in case of errors. But it would
lead to code that is hard to reason about.
So yes, it's a nice catch. It's a bit more verbose to unsubscribe from the
cancelSubscription because it's a one-time observable too. It's not bad, but
it's imperative and the goal of using observables to write less code following
a declarative approach.
Changing the way how you think in order to get the most out of observables
is hard. It's hard even for me. Here we perform a lot of http requests using
observables because we need it to meet the business requirements (feature) and
that's angular way right? But we're doing it wrong. When it comes to multiple
observables in one component, we should compose them somehow together in order
the take advantage of reactive programming (RxJs). But you have to be very
careful with composing observables because it's really hard to get it done or
even maintain it later or read and understand what the code intends to do.
Long story short, when an http observable fulfils, it's not necessary to
unsubscribe. There I can remove the `unsubscribe` from the cancel event if an
error occurs in order to avoid confusion.
The goal of this PR was to cancel the actual http requests when the user
navigates somewhere else. If users are not interested in the pcap panel
anymore, we don't need to wait for the server's answer. It's silly to wait for
the answer for a pcap request when you're on the alerts page.
I'm going to remove the unsubscribe from the cancel observable to avoid
confusion.
I highly recommend to read this article by Ben Lesh (lead dev of the RxJs
team) about it:
https://medium.com/@benlesh/rxjs-dont-unsubscribe-6753ed4fda87
---
[GitHub] metron pull request #1208: METRON-1790: Unsubscribe from every observable in...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1208#discussion_r224353374
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts
---
@@ -35,40 +35,40 @@ export class PcapPanelComponent implements OnInit,
OnDestroy {
pdml: Pdml = null;
pcapRequest: PcapRequest;
resetPaginationForSearch: boolean;
-
- statusSubscription: Subscription;
- cancelSubscription: Subscription;
- submitSubscription: Subscription;
- getSubscription: Subscription;
queryRunning = false;
queryId: string;
progressWidth = 0;
pagination: PcapPagination = new PcapPagination();
savedPcapRequest: {};
errorMsg: string;
cancelConfirmMessage = 'Are you sure want to cancel the running query?';
+ subscriptions: {
+[key: string]: Subscription
+ } = {};
constructor(private pcapService: PcapService) { }
ngOnInit() {
this.pcapRequest = new PcapRequest();
-this.pcapService.getRunningJob().subscribe((statusResponses:
PcapStatusResponse[]) => {
+this.subscriptions['runningJobSubscription'] =
this.pcapService.getRunningJob().subscribe((statusResponses:
PcapStatusResponse[]) => {
--- End diff --
We do here:
https://github.com/ruffle1986/metron/blob/METRON-1790/metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts#L138-L143
I collect every subscription in an object and I go through all of them in
the `unsubscribeAll` method.
---
[GitHub] metron pull request #1219: METRON-1796: [UI] Migrate off moment.js
Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1219#discussion_r223984152 --- Diff: metron-interface/metron-alerts/package.json --- @@ -22,17 +22,17 @@ "@angular/platform-browser": "^6.1.6", "@angular/platform-browser-dynamic": "^6.1.6", "@angular/router": "^6.1.6", +"@ruffle1986/pikaday-time": "^1.6.1", "@types/bootstrap": "^4.1.1", "@types/jquery": "^3.3.4", "ace-builds": "^1.2.6", "ajv": "^6.5.1", "angular-confirmation-popover": "^4.2.0", "bootstrap": "4.0.0-alpha.6", "core-js": "^2.4.1", +"date-fns": "^1.29.0", "font-awesome": "^4.7.0", -"moment": "^2.22.2", "ng2-dragula": "^1.5.0", -"pikaday-time": "^1.6.1", --- End diff -- Alright. I'm going to start a DISCUSS thread on the mailing list about it. ---
[GitHub] metron pull request #1219: METRON-1796: [UI] Migrate off moment.js
Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1219#discussion_r223676139 --- Diff: metron-interface/metron-alerts/package.json --- @@ -22,17 +22,17 @@ "@angular/platform-browser": "^6.1.6", "@angular/platform-browser-dynamic": "^6.1.6", "@angular/router": "^6.1.6", +"@ruffle1986/pikaday-time": "^1.6.1", "@types/bootstrap": "^4.1.1", "@types/jquery": "^3.3.4", "ace-builds": "^1.2.6", "ajv": "^6.5.1", "angular-confirmation-popover": "^4.2.0", "bootstrap": "4.0.0-alpha.6", "core-js": "^2.4.1", +"date-fns": "^1.29.0", "font-awesome": "^4.7.0", -"moment": "^2.22.2", "ng2-dragula": "^1.5.0", -"pikaday-time": "^1.6.1", --- End diff -- > A, simply remove the time picker component If it's not important to let the user choose a time beside the date, then yes, we can get rid of `pikaday-time` and just use `pikaday`. Unfortunately there's a **but** here. :( In order to eliminate moment, we still have to fork pikaday. Here's why: [This](https://github.com/dbushell/Pikaday/pull/721) is a PR against Pikaday's master about removing moment.js from the dependencies. It's already merged but as it turned out, it's not published yet on npm. If you install the latest pikaday from npm, you will get moment as well. :( It's very difficult to follow and chaotic. This is a good example of choosing a 3rd party without proper investigation. And this is also the dangerous part of relying on 3rd party open source modules. [Here's](https://github.com/dbushell/Pikaday/issues/805) an issue to push it, but no answers since 6th of September > B, add a separate one that isn't part of pikaday This would be the best imho because of the chaotic nature of pikaday. But it's hard to find a solution out there I could confidently rely on. The modules I trust are the [bootstrap datepicker](https://ng-bootstrap.github.io/#/components/datepicker/overview) or the [one by material design](https://material.angular.io/components/datepicker/overview). But I have my concerns about these. These are new dependencies and it has to be discussed which one should be picked. Also, introducing a datepicker by angular material would be weird imho, since it's very different from our current design and it wouldn't feel like a cohesive whole. But I'm not in the position to decide. C, simply bring the relevant code into Metron so that if we need to maintain it or make changes we can feely do so? This is why I've forked pikaday-time ---
[GitHub] metron pull request #1219: METRON-1796: [UI] Migrate off moment.js
Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1219#discussion_r223269377 --- Diff: metron-interface/metron-alerts/package.json --- @@ -22,17 +22,17 @@ "@angular/platform-browser": "^6.1.6", "@angular/platform-browser-dynamic": "^6.1.6", "@angular/router": "^6.1.6", +"@ruffle1986/pikaday-time": "^1.6.1", "@types/bootstrap": "^4.1.1", "@types/jquery": "^3.3.4", "ace-builds": "^1.2.6", "ajv": "^6.5.1", "angular-confirmation-popover": "^4.2.0", "bootstrap": "4.0.0-alpha.6", "core-js": "^2.4.1", +"date-fns": "^1.29.0", "font-awesome": "^4.7.0", -"moment": "^2.22.2", "ng2-dragula": "^1.5.0", -"pikaday-time": "^1.6.1", --- End diff -- Correct. ---
[GitHub] metron pull request #1219: METRON-1796: [UI] Migrate off moment.js
Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1219#discussion_r223030359 --- Diff: metron-interface/metron-alerts/package.json --- @@ -22,17 +22,17 @@ "@angular/platform-browser": "^6.1.6", "@angular/platform-browser-dynamic": "^6.1.6", "@angular/router": "^6.1.6", +"@ruffle1986/pikaday-time": "^1.6.1", "@types/bootstrap": "^4.1.1", "@types/jquery": "^3.3.4", "ace-builds": "^1.2.6", "ajv": "^6.5.1", "angular-confirmation-popover": "^4.2.0", "bootstrap": "4.0.0-alpha.6", "core-js": "^2.4.1", +"date-fns": "^1.29.0", "font-awesome": "^4.7.0", -"moment": "^2.22.2", "ng2-dragula": "^1.5.0", -"pikaday-time": "^1.6.1", --- End diff -- > Why is pikaday-time suddenly an issue? I agree with you to not use smaller individual projects like `pikaday-time`. `pikaday-time` is introduced in the project by @iraghumitra almost a year ago. The goal wasn't replacing `pikaday-time` with a better well-supported library but eliminating `moment.js` because of the aforementioned reasons in the description. > I am concerned about this pikaday dependency. I would rather see us depending on larger, community supported projects like https://momentjs.com/, rather than smaller, individual supported projects like @owenmean/pikaday (or even your own fork @ruffle1986/pikaday-time). `moment.js` is a very popular and great library to manipulate date and time. `date-fns` has the exact same purpose. `pikaday` is a standalone calendar user interface component. `pikaday-time` is an extension of `pikaday` with additional fields to select time, not just date. `moment.js` is an optional dependency of `pikaday` (! not `pikaday-time`). it works perfectly fine without `moment.js`. But if it is convenient to use `pikaday` with `moment`, it's fine then, you can use it by calling `pikaday`'s `getMoment` method if needed. `pikaday-time` is just a fork of `pikaday` and the author of `pikaday-time` made a mistake by setting `moment.js` as a dependency of `pikaday-time` and every time we install `pikaday-time` we install `moment.js` too. If `moment.js` is installed in the `node_modules` folder `pikaday` will use it therefore it will be part of the bundle. ---
[GitHub] metron pull request #1217: METRON-1749: Update Angular to latest release in ...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1217#discussion_r222622136
--- Diff: metron-interface/metron-config/package.json ---
@@ -7,10 +7,11 @@
},
"angular-cli": {},
"scripts": {
-"build": "./node_modules/angular-cli/bin/ng build -prod",
+"build": "./node_modules/@angular/cli/bin/ng build --prod",
--- End diff --
It can also be written as `ng build --prod`
---
[GitHub] metron pull request #1217: METRON-1749: Update Angular to latest release in ...
Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1217#discussion_r222619789 --- Diff: metron-interface/metron-config/package.json --- @@ -10,7 +10,7 @@ "build": "./node_modules/angular-cli/bin/ng build -prod", --- End diff -- it can also be written as `ng build -prod` ---
[GitHub] metron pull request #1219: METRON-1796: [UI] Migrate off moment.js
Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1219#discussion_r222315533 --- Diff: metron-interface/metron-alerts/package.json --- @@ -46,9 +46,7 @@ "@types/ace": "0.0.32", "@types/es6-promise": "0.0.33", "@types/jasmine": "2.5.38", -"@types/moment": "^2.13.0", "@types/node": "~6.0.60", -"@types/pikaday-time": "^1.4.2", --- End diff -- The reason why I removed this is because moment.js is the dependency of this to make it able to add the type `moment` to the return value of the `getMoment` method. Therefore moment.js has to be inside the node_modules folder but we wan't to avoid it to be there. I don't see any benefits of having the type definitions of pikaday-time in the code base. ---
[GitHub] metron pull request #1219: METRON-1796: [UI] Migrate off moment.js
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1219#discussion_r222317156
--- Diff: metron-interface/metron-alerts/src/app/utils/utils.spec.ts ---
@@ -0,0 +1,215 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {Utils} from './utils'
+
+describe('timeRangeToDisplayStr', () => {
--- End diff --
[timeRangeToDisplayStr](https://github.com/apache/metron/blob/master/metron-interface/metron-alerts/src/app/utils/utils.ts#L77-L206)
relies heavily on moment.js so before touching the implementation I covered it
with unit tests to make sure that the util produces the exact same result after
the removal of moment.js.
---
[GitHub] metron pull request #1219: METRON-1796: [UI] Migrate off moment.js
Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1219#discussion_r222308215 --- Diff: metron-interface/metron-alerts/package.json --- @@ -22,17 +22,17 @@ "@angular/platform-browser": "^6.1.6", "@angular/platform-browser-dynamic": "^6.1.6", "@angular/router": "^6.1.6", +"@ruffle1986/pikaday-time": "^1.6.1", "@types/bootstrap": "^4.1.1", "@types/jquery": "^3.3.4", "ace-builds": "^1.2.6", "ajv": "^6.5.1", "angular-confirmation-popover": "^4.2.0", "bootstrap": "4.0.0-alpha.6", "core-js": "^2.4.1", +"date-fns": "^1.29.0", "font-awesome": "^4.7.0", -"moment": "^2.22.2", "ng2-dragula": "^1.5.0", -"pikaday-time": "^1.6.1", --- End diff -- pikaday-time is an extension of [Pikaday](https://github.com/dbushell/Pikaday). Pikaday is a great library with zero dependency. Pikaday-time is extends its functionality with time selection. However moment is just an optional dependency of Pikaday, there's an unpleasant side-effect in Pikaday-time where moment.js is set as a dependency in the package.json. So everytime we install pikaday-time, moment.js will be installed as well. [I've tried to reach out the maintainer of the pikaday-time](https://github.com/owenmead/Pikaday/issues/60) library but he's not so active on Github and the library is no longer maintained anyway so it makes things a bit complicated. Hopefully we'll manage this and a patch for this will be introduced shortly. Until then, [I've forked the pikaday-time repo](https://github.com/owenmead/Pikaday/compare/master...ruffle1986:master), made the changes and [published it on npm under my name](https://www.npmjs.com/package/@ruffle1986/pikaday-time). Originally I wanted to published it under [hortonworks](https://www.npmjs.com/org/hortonworks) but I don't know who have access to give me publish rights. The only change I made is setting moment.js as a `peerDependency` insteadOf setting it as an `optionalDependency`. Don't let the name `optionalDependency` mislead you. [It's basically a dependency but if it's cannot be found on npm on any given registries, npm install doesn't fail](https://docs.npmjs.com/files/package.json#optionaldependencies). As soon as this issue is fixed and published on npm, we can remove the scoped pikaday-time and switch back to the original package. ---
[GitHub] metron pull request #1219: METRON-1796: [UI] Migrate off moment.js
GitHub user ruffle1986 opened a pull request: https://github.com/apache/metron/pull/1219 METRON-1796: [UI] Migrate off moment.js ## Contributor Comments [DISCUSS] thread on the dev mailing list: https://lists.apache.org/thread.html/2e4fafa4256ce14ebcd4433420974e24962884204418ade51f0e3bfb@%3Cdev.metron.apache.org%3E Original ASF Jira ticket: https://issues.apache.org/jira/browse/METRON-1796 The purpose of this PR is the complete removal of [moment.js](http://momentjs.com/) from the code base and replacing it with either native functions or [date-fns](https://date-fns.org/), another date manipulation library. **Motivation** In the Metron Alerts UI, we use a few functions only from moment.js to deal with time like formatting or displaying the relative time from "now". In order to do that, we have to import the entire library which causes a significant footprint in the compiled production bundle. Sometimes they can be replaced with native built-in solutions, sometimes they cannot but we can use date-fns which has a smaller size comparing to moment.js As of date-fns v2 is in alpha phase, I rather chose the latest stable version which is 1.29.0. Tree-shaking is currently not supported in version 1 but [it will be in version 2](https://date-fns.org/v2.0.0-alpha.9/docs/ECMAScript-Modules). So once it's released and we can upgrade to it, we can see more size loss in the bundle size. Angular uses Webpack under the hood so if you're interested in what tree-shaking is and how Webpack does it, [follow this link](https://webpack.js.org/guides/tree-shaking/) for further details. **Changes included** - Every code used moment.js is replaced with a built-in function or the appropriate function from date-fns. I highly recommend you to go through all the commits I've made one by one. They're straightforward and easier to understand which parts of the app are affected and in what way. **Testing the bundle file size** 1. Checkout `master` 1. Go to `metron-interface/metron-alerts` 1. Make sure you have the latest packages in the `node_modules` folder via `npm ci` 1. Run `npm run build` 1. Take a look at the size of the `main.js` file 1. Checkout `ruffle1986/METRON-1796` 1. Run `npm ci` (this will install the date-fns library and remove moment.js) 1. Run `npm run build` 1. Take a look at the size of the `main.js` file and compare it to the number you've got in step 4 Here's the output of `npm run build` on the `master` branch:  And here's the output on this branch:  As you can see, by removing moment.js, we could reduce the bundle size to 1.65 MB which is **15.6%** comparing to the bundle with moment.js. Not so significant but still. You've probably noticed the warning message on the second picture below the output of the build process. This is the result of compiling `pikaday-time` into our bundle via Angular. For the record, it's totally fine. Moment.js is just an optional dependency of [Pikaday](https://dbushell.com/Pikaday/), it works perfectly fine without it. This warning message is there because Pikaday [checks whether it's a Node.js environment and since it is, it wants to load moment from the node_modules folder](https://github.com/dbushell/Pikaday/blob/master/pikaday.js#L12-L16). However Pikaday doesn't throw and fails silently, the Angular compiler is clever enough to catch this and kindly warn you about it. But it doesn't cause any negative effect in the Metron UI because (again) moment.js is not a required dependency of Pikaday. ## Pull Request Checklist Thank you for submitting a contribution to Apache Metron. Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions. Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides. In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following: ### For all changes: - [X] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel). - [X] Does your PR title start with METRON- where is the JIRA number you are trying to resolve? Pay particular
[GitHub] metron pull request #1199: METRON-1760: Kill PCAP job should prompt for conf...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1199#discussion_r218451156
--- Diff: metron-interface/metron-alerts/package-lock.json ---
@@ -5808,6 +5808,15 @@
"integrity": "sha1-SlKCrBZHKek2Gbz9OtFR+BfOkfU=",
"dev": true
},
+"angular-confirmation-popover": {
+ "version": "4.2.0",
+ "resolved":
"https://registry.npmjs.org/angular-confirmation-popover/-/angular-confirmation-popover-4.2.0.tgz;,
+ "integrity":
"sha512-ItCPzV52user93NRk9rF4Rp8NpawBWJdkNf8+6lH//f5i/N5HY0Aq5Hcch3xk19h9P48k0WZnfwOQL181xe4MQ==",
+ "requires": {
+"positioning": "^1.3.1",
--- End diff --
@nickwallen According to semantic versioning and how npm handles these, The
`^` sign at the beginning of the version means that any other "minor" updates
of Positioning are accepted by the popover but not a major change.
1.4.0 is a minor update comparing to 1.3.1 since only the second number has
been changed so npm installed and stored 1.4.0 which is the latest version and
also the latest minor version of the package within version 1.
---
[GitHub] metron issue #1199: METRON-1760: Kill PCAP job should prompt for confirmatio...
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1199 @nickwallen I added the instructions on how to work with pcap data, but there's another way to slow things down. You can emulate a very slow internet connection on the network tab if you're using Google Chrome: https://developers.google.com/web/tools/chrome-devtools/network-performance/reference#throttling ---
[GitHub] metron pull request #1199: METRON-1760: Kill PCAP job should prompt for conf...
GitHub user ruffle1986 opened a pull request: https://github.com/apache/metron/pull/1199 METRON-1760: Kill PCAP job should prompt for confirmation ## Contributor Comments This PR introduces a so-called `pop confirm` component appearing next to "kill query button" when the user presses it. We thought it would be better to provide a middle step between the user interaction and the http request since the querying process can be a time consuming process and the cancel button can be pressed accidentally giving the user a hard time. You can find the original JIRA ticket [here](https://issues.apache.org/jira/browse/METRON-1760). I decided to not create this floating element from scratch but rather install a [component](https://www.npmjs.com/package/angular-confirmation-popover) from npm which seemed to fulfil the requirements. It uses Bootstrap behind the scenes but I slightly modified the css to make it fit to the current design. The module has the appropriate [license](https://github.com/mattlewis92/angular-confirmation-popover/blob/master/package.json#L55) and the number of potential vulnerabilities, according to `npm audit`, is zero. **Changes included:** - Use the new component as a directive on the kill button in [pcap-panel.component.html](https://github.com/ruffle1986/metron/blob/METRON-1760/metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.html#L23). It's reusable and you can use it on any other elements as you wish by setting the `mwlConfirmationPopover ` directive. You can also add [different attributes](https://github.com/ruffle1986/metron/blob/METRON-1760/metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.html#L24-L31) to configure the behaviour of the component. I wouldn't go into details since they're pretty straightforward. Check out the [docs](https://mattlewis92.github.io/angular-confirmation-popover/docs/)! - The "killing query process" had been already tested before on the ui so I had to slightly change the test units accordingly. You cannot expect the same result unless you click on the "Yes" button on the pop confirm component. - [Override](https://github.com/ruffle1986/metron/blob/METRON-1760/metron-interface/metron-alerts/src/confirm-popover.scss) Bootstrap's styles to make it fit to the current (dark) design. - Update the package lock file due to the new dependency. **Testing:** Go to the PCAP tab and start a new query by clicking on the magnifier at the right side of the filter panel. The progress bar is supposed to appear with a cancel button on the right (a button with an "X" on it). During the process you should be able to click on the cancel button and the pop confirm component should appear next to the button to prompt the user to solidify his/her decision or give the opportunity to change his/her mind. ## Pull Request Checklist Thank you for submitting a contribution to Apache Metron. Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions. Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides. In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following: ### For all changes: - [X] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel). - [X] Does your PR title start with METRON- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [X] Has your PR been rebased against the latest commit within the target branch (typically master)? ### For code changes: - [X] Have you included steps to reproduce the behavior or problem that is being changed or addressed? - [X] Have you included steps or a guide to how the change may be verified and tested manually? - [X] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via: ``` mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh ``` - [X] Have you written or updated unit tests and or integration tests to verify your changes? - [X] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [X] Have you verified the basic f
[GitHub] metron pull request #1182: METRON-1760: show a confirmation popover before k...
Github user ruffle1986 closed the pull request at: https://github.com/apache/metron/pull/1182 ---
[GitHub] metron issue #1182: METRON-1760: show a confirmation popover before killing ...
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1182 Closing due to incorrect PR conventions. ---
[GitHub] metron pull request #1096: METRON-1476: Update to Angular 6.1.3
Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1096#discussion_r215171209 --- Diff: metron-interface/metron-alerts/README.md --- @@ -45,7 +45,7 @@ Alerts that are contained in a a meta alert are generally excluded from search r * The Management UI should be installed (which includes [Express](https://expressjs.com/)) * The alerts can be populated using Full Dev or any other setup * UI is developed using angular4 and uses angular-cli -* node.JS >= 7.8.0 +* nvm (or a similar node verison manager) should be installed. The node version required for this project is listed in the [.nvmrc](https://github.com/creationix/nvm#nvmrc) file. --- End diff -- We should also update the line above (line 47.): > UI is developed using angular4 and uses angular-cli to: > UI is developed using angular 6 and uses angular-cli ---
[GitHub] metron pull request #1096: METRON-1476: Update to Angular 6.1.3
Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1096#discussion_r215170216 --- Diff: metron-interface/metron-alerts/README.md --- @@ -45,7 +45,7 @@ Alerts that are contained in a a meta alert are generally excluded from search r * The Management UI should be installed (which includes [Express](https://expressjs.com/)) * The alerts can be populated using Full Dev or any other setup * UI is developed using angular4 and uses angular-cli -* node.JS >= 7.8.0 +* nvm (or a similar node verison manager) should be installed. The node version required for this project is listed in the [.nvmrc](https://github.com/creationix/nvm#nvmrc) file. --- End diff -- I prefer following the instructions in the repo's readme to install nvm. btw, yes you can install it via homebrew on a mac. ---
[GitHub] metron issue #1180: METRON-1759: PCAP UI: Removing wrong Input annotations f...
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1180 +1. Nice catch! ð» ---
[GitHub] metron pull request #1182: METRON-1760: show a confirmation popover before k...
GitHub user ruffle1986 opened a pull request: https://github.com/apache/metron/pull/1182 METRON-1760: show a confirmation popover before killing the pcap query ## Contributor Comments Currently, if we have a running pcap query and we click on the "cancel" button, the request goes to the server immediately. Since it could take long, it's very annoying if the user clicks on the cancel button accidentally when the query is so close to be fulfilled. In this patch, the user is asked whether she really wants to cancel the running pcap query by a confirmation popover appearing next to the cancel button. FYI: in this PR, I'm introducing a new dependency: https://github.com/mattlewis92/angular-confirmation-popover#readme ## Pull Request Checklist Thank you for submitting a contribution to Apache Metron. Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions. Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides. In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following: ### For all changes: - [X] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel). - [X] Does your PR title start with METRON- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [X] Has your PR been rebased against the latest commit within the target branch (typically master)? ### For code changes: - [X] Have you included steps to reproduce the behavior or problem that is being changed or addressed? - [X] Have you included steps or a guide to how the change may be verified and tested manually? - [X] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via: ``` mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh ``` - [X] Have you written or updated unit tests and or integration tests to verify your changes? - [X] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [X] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent? ### For documentation related changes: - [ ] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via `site-book/target/site/index.html`: ``` cd site-book mvn site ``` Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ruffle1986/metron METRON-1760 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/1182.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1182 commit b5788fb1f892012d4cd105fc8c23667fd7c8df6f Author: Shane Ardell Date: 2018-06-27T13:08:01Z update angular to current stable version commit 4f1ee4fea99cc154ee0a328a349abab58c0d67a2 Author: Shane Ardell Date: 2018-07-05T10:45:58Z remove module file extension commit cd5942a11c25e8ccb1be98919e3c4883823f2f59 Author: Shane Ardell Date: 2018-07-05T11:09:53Z add http interceptor for default headers commit fa40b49f9a6d2aa4b36a9266e4b72b28a5c26ecb Author: Shane Ardell Date: 2018-07-05T12:01:16Z update configurations commit 86bfaa30c89b24474fbfaed118c3b35c483c19e3 Author: Shane Ardell Date: 2018-07-05T12:02:21Z update dependencies commit 1abd618b5f2b53381feb23ba12ccb1fa282da297 Author: Shane Ardell Date: 2018-07-05T12:03:06Z update services to use HttpClient and rxjs changes commit c7e7c95fd84618218b786d8f8ecad3d660434318 Author: Shane Ardell Date: 2018-07-05T15:50:24Z merge master and resolve conflicts in view and login specs commit d7e93fc6558903aa95d
[GitHub] metron issue #1096: METRON-1476: Update angular
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1096 It has turned out that Angular's changed the way how it renders templates by default in version 6. Earlier, it preserved whitespaces by default but in version 6 the `preservedWhitespace` option is false by default when using the aot (head of time) compiler. Actually, it's a good thing because it reduces the size of the production bundle but during the regression test, we noticed a few differences in the layout before and after the version upgrade. So since we've been relying on whitespaces, we have to set `preserveWhitespace` true. https://github.com/sardell/metron/pull/10 ---
[GitHub] metron pull request #1096: METRON-1476: Update angular
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1096#discussion_r213278817
--- Diff:
metron-interface/metron-alerts/src/app/alerts/alerts-list/table-view/table-view.component.spec.ts
---
@@ -42,17 +42,18 @@ describe('TableViewComponent', () => {
let fixture: ComponentFixture;
beforeEach(async(() => {
-// FIXME: mock all the unnecessary dependencies
+// FIXME: mock all the unnecessary dependencies
TestBed.configureTestingModule({
- imports: [ HttpModule ],
+ imports: [ HttpClientModule ],
providers: [
SearchService,
UpdateService,
GlobalConfigService,
MetaAlertService,
MetronDialogBox,
+HttpClient,
--- End diff --
Isn't it enough to import the `HttpClientModule` only? Is it necessary to
explicitly set `HttpClient` as a provider?
As far as I know, you only have to set the HttpClient when you want to set
a mock instead of the original http module.
---
[GitHub] metron pull request #1096: METRON-1476: Update angular
Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1096#discussion_r213283494 --- Diff: metron-interface/metron-alerts/.nvmrc --- @@ -0,0 +1 @@ +v9.11.1 --- End diff -- This version of node includes npm v5.6.0 which hasn't got `ci` command yet. Should `npm` be updated manually to the latest version? If yes, we should add the update command to the maven config as well. ---
[GitHub] metron issue #1172: METRON-1724: Date/time validation missing in PCAP query
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1172 It's not just simply about sharing the same validator function. It's more complicated than that. Here's a possible fix for that: https://github.com/tiborm/metron/pull/11 ---
[GitHub] metron issue #1172: METRON-1724: Date/time validation missing in PCAP query
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1172 I think the problem is that we use the same validator function for both the start and the end date. So after putting it into an invalid state (step 2), however you add a valid time to the start date, the validator function will complain because the end time is still in an invalid state. That's why the start time is still marked in red. ---
[GitHub] metron pull request #1172: METRON-1724: Date/time validation missing in PCAP...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1172#discussion_r212205588
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.ts
---
@@ -15,63 +15,116 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {Component, Input, Output, EventEmitter, OnInit, OnChanges,
SimpleChanges} from '@angular/core';
+import {Component, Input, Output, EventEmitter, OnChanges, SimpleChanges}
from '@angular/core';
+import { FormGroup, FormControl, Validators, ValidationErrors } from
'@angular/forms';
+
import * as moment from 'moment/moment';
import { DEFAULT_TIMESTAMP_FORMAT } from '../../utils/constants';
import { PcapRequest } from '../model/pcap.request';
+const DEFAULT_END_TIME = new Date();
--- End diff --
Yes, probably that would be the best because somehow, this request related
stuff doesn't want to fit in with the filter component :D
---
[GitHub] metron pull request #1172: METRON-1724: Date/time validation missing in PCAP...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1172#discussion_r212205052
--- Diff: metron-interface/metron-alerts/src/app/pcap/model/pcap.request.ts
---
@@ -17,13 +17,13 @@
*/
export class PcapRequest {
- startTimeMs: number = 0;
- endTimeMs: number = 15;
- ipSrcAddr: string = '';
- ipSrcPort: number;
- ipDstAddr: string = '';
- ipDstPort: number;
- protocol: string = '';
- packetFilter: string = '';
- includeReverse: boolean = false;
+ startTimeMs = 0;
+ endTimeMs = 15;
--- End diff --
but, again. it's the type of the request which is fine. we don't have to
use the same values to set the state of the filter ui component. it can be a
totally different type. When it comes to sending the request to the server (or
getting a response from it), this is the proper place where we're supposed to
deal with request types.
---
[GitHub] metron pull request #1172: METRON-1724: Date/time validation missing in PCAP...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1172#discussion_r212204263
--- Diff: metron-interface/metron-alerts/src/app/pcap/model/pcap.request.ts
---
@@ -17,13 +17,13 @@
*/
export class PcapRequest {
- startTimeMs: number = 0;
- endTimeMs: number = 15;
- ipSrcAddr: string = '';
- ipSrcPort: number;
- ipDstAddr: string = '';
- ipDstPort: number;
- protocol: string = '';
- packetFilter: string = '';
- includeReverse: boolean = false;
+ startTimeMs = 0;
+ endTimeMs = 15;
--- End diff --
It's not necessary to set these values here but I'm afraid the server
relies on it as default values. So it might crash if we send it undefined
values instead of magic numbers.
---
[GitHub] metron pull request #1172: METRON-1724: Date/time validation missing in PCAP...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1172#discussion_r211972645
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.ts
---
@@ -15,63 +15,116 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {Component, Input, Output, EventEmitter, OnInit, OnChanges,
SimpleChanges} from '@angular/core';
+import {Component, Input, Output, EventEmitter, OnChanges, SimpleChanges}
from '@angular/core';
+import { FormGroup, FormControl, Validators, ValidationErrors } from
'@angular/forms';
+
import * as moment from 'moment/moment';
import { DEFAULT_TIMESTAMP_FORMAT } from '../../utils/constants';
import { PcapRequest } from '../model/pcap.request';
+const DEFAULT_END_TIME = new Date();
--- End diff --
They belong to the file's scope.
They could collide with others but we can import them in this way:
```
import { DEFAULT_END_TIME as DEFAULT_PCAP_FILTER_END_TIME } from
'./pcap-filter.component';
```
But I can hardly imagine that we would use them somewhere else out of this
scope.
---
[GitHub] metron pull request #1172: METRON-1724: Date/time validation missing in PCAP...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1172#discussion_r211970078
--- Diff: metron-interface/metron-alerts/src/app/pcap/model/pcap.request.ts
---
@@ -17,13 +17,13 @@
*/
export class PcapRequest {
- startTimeMs: number = 0;
- endTimeMs: number = 15;
- ipSrcAddr: string = '';
- ipSrcPort: number;
- ipDstAddr: string = '';
- ipDstPort: number;
- protocol: string = '';
- packetFilter: string = '';
- includeReverse: boolean = false;
+ startTimeMs = 0;
+ endTimeMs = 15;
--- End diff --
Why the magic number?
We use this value to produce a formatted date string by using the Date
constructor. But
```js
new Date(15) // -> Invalid Date
```
so we have to do extra checks to avoid showing "Invalid Date" in the date
range selector:
https://github.com/tiborm/metron/blob/METRON-1724/metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.ts#L59
which makes it hard to understand and maintain the code.
Using magic numbers is a bad practice because it's hard to understand their
meaning. We should add a hint as a code comment at least. Or use another, more
straightforward value.
My first thought was `Date.now()` but it would introduce side effects so I
let it go.
What do you think?
---
[GitHub] metron pull request #1172: METRON-1724: Date/time validation missing in PCAP...
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1172#discussion_r211960374
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.ts
---
@@ -15,63 +15,116 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {Component, Input, Output, EventEmitter, OnInit, OnChanges,
SimpleChanges} from '@angular/core';
+import {Component, Input, Output, EventEmitter, OnChanges, SimpleChanges}
from '@angular/core';
+import { FormGroup, FormControl, Validators, ValidationErrors } from
'@angular/forms';
+
import * as moment from 'moment/moment';
import { DEFAULT_TIMESTAMP_FORMAT } from '../../utils/constants';
import { PcapRequest } from '../model/pcap.request';
+const DEFAULT_END_TIME = new Date();
--- End diff --
For the record, in this case, it would be ok to put these things inside the
class definition because they're connected to the pcap filter so they would be
hardly reused by other components, but who knows. It's hard to predict.
I'm not sure about how these kind of things work in Java or other OOP heavy
programming languages but Javascript (NodeJS) doesn't enforce us to put
constants and static functions inside a class.
The functions are static pure functions. They have nothing to do with the
class' instance (this) so they should not be a private or a public class
methods. they could be a static functions (a property of the class object
itself). From testing perspective, I would keep it as is but export them of
course so we wouldn't have to create a new instance of the component to test
them. The best would be if they were replaced into a separate file in the
appropriate folder based on whether it's shared across other components.
---
[GitHub] metron issue #1158: METRON-1733: PCAP UI - PCAP queries don't work on Safari
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1158 I think it's just enough the trigger a travis rebuild to make it pass. ---
[GitHub] metron issue #1156: METRON-1702: Reload a running job in the UI
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1156 @merrimanr Looks good to me. ---
[GitHub] metron pull request #1156: METRON-1702: Reload a running job in the UI
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1156#discussion_r208968690
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts
---
@@ -15,22 +15,22 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import { Component, Input } from '@angular/core';
+import { Component, OnInit, Input } from '@angular/core';
import { PcapService } from '../service/pcap.service';
import { PcapStatusResponse } from '../model/pcap-status-response';
import { PcapRequest } from '../model/pcap.request';
import { Pdml } from '../model/pdml';
import { Subscription } from 'rxjs/Rx';
import { PcapPagination } from '../model/pcap-pagination';
-import {RestError} from "../../model/rest-error";
+import { RestError } from "../../model/rest-error";
@Component({
selector: 'app-pcap-panel',
templateUrl: './pcap-panel.component.html',
styleUrls: ['./pcap-panel.component.scss']
})
-export class PcapPanelComponent {
+export class PcapPanelComponent implements OnInit {
--- End diff --
I've already done it here in this commit:
https://github.com/apache/metron/commit/14dcb2d90581835d8206c65918c24e8cb04bfd06
It's already merged back into the feature branch and it implements the
destroy interface and I also refactored it a bit to make it more maintainable.
We both touched the same lines so these conflicts have to be resolved before
merging it back.
---
[GitHub] metron pull request #1156: METRON-1702: Reload a running job in the UI
Github user ruffle1986 commented on a diff in the pull request:
https://github.com/apache/metron/pull/1156#discussion_r208946955
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts
---
@@ -15,22 +15,22 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import { Component, Input } from '@angular/core';
+import { Component, OnInit, Input } from '@angular/core';
import { PcapService } from '../service/pcap.service';
import { PcapStatusResponse } from '../model/pcap-status-response';
import { PcapRequest } from '../model/pcap.request';
import { Pdml } from '../model/pdml';
import { Subscription } from 'rxjs/Rx';
import { PcapPagination } from '../model/pcap-pagination';
-import {RestError} from "../../model/rest-error";
+import { RestError } from "../../model/rest-error";
@Component({
selector: 'app-pcap-panel',
templateUrl: './pcap-panel.component.html',
styleUrls: ['./pcap-panel.component.scss']
})
-export class PcapPanelComponent {
+export class PcapPanelComponent implements OnInit {
--- End diff --
@merrimanr You should also implement the OnDestroy interface and
unsubscribe from all the observables in the `ngOnDestroy` method. Especially
from the polling observable. By doing this, you can make sure that the
component won't continue the polling for the job's status when the user
navigates somewhere else from the pcap tab. If you unsubscribe in the
destructor method, it'll stop the pending http requests immediately as well
when the route changes. Not to mention freeing up the memory when the component
is not rendered anymore.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Github user ruffle1986 commented on the issue: https://github.com/apache/metron/pull/1122 @merrimanr Yes. accidentally, we forgot to change `fdescribe` for `describe` in the pcap-list component's test file before performing the commit. It means that the runner runs those tests only. https://github.com/sardell/metron/blob/METRON-1683/metron-interface/metron-alerts/src/app/pcap/pcap-list/pcap-list.component.spec.ts#L46 cc @sardell ---
[GitHub] metron pull request #1133: Pcap: Add source and destination port validation
Github user ruffle1986 closed the pull request at: https://github.com/apache/metron/pull/1133 ---
[GitHub] metron pull request #1133: Bug-107904 Pcap: Add source and destination port ...
GitHub user ruffle1986 opened a pull request: https://github.com/apache/metron/pull/1133 Bug-107904 Pcap: Add source and destination port validation ## Contributor Comments From now on the users are able to push the submit button if they set the port number between 0 and 9. If the given port includes invalid characters of is out of range, the submit button is disabled and the field is marked as invalid. ## Pull Request Checklist Thank you for submitting a contribution to Apache Metron. Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions. Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides. In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following: ### For all changes: - [X] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel). - [ ] Does your PR title start with METRON- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)? ### For code changes: - [ ] 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 manually? - [ ] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via: ``` mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh ``` - [X] Have you written or updated unit tests and or integration tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [X] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent? ### For documentation related changes: - [ ] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via `site-book/target/site/index.html`: ``` cd site-book mvn site ``` Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ruffle1986/metron BUG-107904-refactor-pcap-filter-form-validation Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/1133.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1133 commit 872d1b1ee13e358c18956945d71d3667d19fca8a Author: merrimanr Date: 2018-04-12T14:57:48Z Merge branch 'pcap-front' of https://github.com/simonellistonball/metron into pcaprest Conflicts: metron-interface/metron-alerts/src/app/app.module.ts commit b1b6a7dabea1a1d0d132482c8d97af29c0ac2683 Author: merrimanr Date: 2018-04-13T15:00:15Z initial commit Conflicts: metron-interface/metron-rest/src/main/java/org/apache/metron/rest/MetronRestApplication.java metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/PcapQueryController.java metron-interface/metron-rest/src/main/java/org/apache/metron/rest/util/pcapQueryThread.java commit 55cf2d945a4fcff1e7e2e47a234037ed6f394b2e Author: merrimanr Date: 2018-04-18T15:52:56Z added license headers commit 70696d047c6ef4b8ce5fcda03588474ff5b2c506 Author: tiborm Date: 2018-07-11T14:58:56Z METRON-1662: adding download button to pcap tab commit 4a83d46259736d8c721fb9f640b6ad522f91a559 Author: tiborm Date: 2018-07-12T07:09:14Z METRON-1662: removing unused function from the first iteration commit 9f57724597240a05d1caeec73882a4ed50aadd9a Author: tiborm Date: 2018-07-12T08:36:15Z METRON-1662: changing download button label from pd
