[GitHub] metron issue #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread ottobackwards
Github user ottobackwards commented on the issue:

https://github.com/apache/metron/pull/481
  
+1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread ottobackwards
Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/481#discussion_r127087711
  
--- Diff: 
metron-platform/metron-writer/src/main/java/org/apache/metron/writer/bolt/BulkMessageWriterBolt.java
 ---
@@ -92,24 +177,58 @@ public void prepare(Map stormConf, TopologyContext 
context, OutputCollector coll
   configurationTransformation = x -> x;
 }
 try {
-  bulkMessageWriter.init(stormConf,
- context,
- configurationTransformation.apply(new 
IndexingWriterConfiguration(bulkMessageWriter.getName(),
- getConfigurations()))
-);
+  WriterConfiguration writerconf = configurationTransformation.apply(
+  new IndexingWriterConfiguration(bulkMessageWriter.getName(), 
getConfigurations()));
+  if (defaultBatchTimeout == 0) {
+//This means getComponentConfiguration was never called to 
initialize defaultBatchTimeout,
+//probably because we are in a unit test scenario.  So calculate 
it here.
+BatchTimeoutHelper timeoutHelper = new 
BatchTimeoutHelper(writerconf::getAllConfiguredTimeouts, batchTimeoutDivisor);
+defaultBatchTimeout = timeoutHelper.getDefaultBatchTimeout();
+  }
+  writerComponent.setDefaultBatchTimeout(defaultBatchTimeout);
+  bulkMessageWriter.init(stormConf, context, writerconf);
 } catch (Exception e) {
   throw new RuntimeException(e);
 }
   }
 
+  /**
+   * Used only for unit testing.
+   */
--- End diff --

let the intellisense fall where it may then


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #649: METRON-1035 Added SUM to the rules triage aggregat...

2017-07-12 Thread simonellistonball
GitHub user simonellistonball opened a pull request:

https://github.com/apache/metron/pull/649

METRON-1035 Added SUM to the rules triage aggregation docs

## Contributor Comments

Quick doc fix verified against code. 

## Pull Request Checklist


### 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 documentation related changes:
- [x] 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/simonellistonball/incubator-metron METRON-1035

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/metron/pull/649.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 #649


commit d2604086c52f00656ffab23b8b4eab1d30aeecee
Author: Simon Elliston Ball 
Date:   2017-07-12T22:12:31Z

Added SUM to the rules triage aggregation docs




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron issue #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread mattf-horton
Github user mattf-horton commented on the issue:

https://github.com/apache/metron/pull/481
  
@ottobackwards , thanks for looking at this.  I did not see those error 
behaviors while running integration tests.  Please tell me the environment and 
the exact invocation you used.  Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread mattf-horton
Github user mattf-horton commented on a diff in the pull request:

https://github.com/apache/metron/pull/481#discussion_r127081873
  
--- Diff: 
metron-platform/metron-writer/src/main/java/org/apache/metron/writer/bolt/BulkMessageWriterBolt.java
 ---
@@ -92,24 +177,58 @@ public void prepare(Map stormConf, TopologyContext 
context, OutputCollector coll
   configurationTransformation = x -> x;
 }
 try {
-  bulkMessageWriter.init(stormConf,
- context,
- configurationTransformation.apply(new 
IndexingWriterConfiguration(bulkMessageWriter.getName(),
- getConfigurations()))
-);
+  WriterConfiguration writerconf = configurationTransformation.apply(
+  new IndexingWriterConfiguration(bulkMessageWriter.getName(), 
getConfigurations()));
+  if (defaultBatchTimeout == 0) {
+//This means getComponentConfiguration was never called to 
initialize defaultBatchTimeout,
+//probably because we are in a unit test scenario.  So calculate 
it here.
+BatchTimeoutHelper timeoutHelper = new 
BatchTimeoutHelper(writerconf::getAllConfiguredTimeouts, batchTimeoutDivisor);
+defaultBatchTimeout = timeoutHelper.getDefaultBatchTimeout();
+  }
+  writerComponent.setDefaultBatchTimeout(defaultBatchTimeout);
+  bulkMessageWriter.init(stormConf, context, writerconf);
 } catch (Exception e) {
   throw new RuntimeException(e);
 }
   }
 
+  /**
+   * Used only for unit testing.
+   */
--- End diff --

Hm.  With respect, this one I don't really agree with.  It's just a 
polymorphism of the regular prepare() method.  Since its signature differs from 
the "regular" prepare() method, system code will never call it.  It is 
documented, at javadoc level, as being only for use in unit testing, which is 
by definition code-level.  Those who aren't willing to look at code shouldn't 
be using it, and indeed will have a difficult time finding it at all.  I'd 
rather let the polymorphism be evident.

I will, however, add further comment that this is "used only for unit 
testing, with injection of a FakeClock for mocking passage of time in a 
controlled way, to test queue timeout behavior in the WriterComponent."  Would 
that be okay?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread mattf-horton
Github user mattf-horton commented on a diff in the pull request:

https://github.com/apache/metron/pull/481#discussion_r127077516
  
--- Diff: 
metron-platform/metron-enrichment/src/test/java/org/apache/metron/enrichment/bolt/BulkMessageWriterBoltTest.java
 ---
@@ -164,4 +174,100 @@ public void test() throws Exception {
 verify(outputCollector, times(1)).emit(eq(Constants.ERROR_STREAM), 
any(Values.class));
 verify(outputCollector, times(1)).reportError(any(Throwable.class));
   }
+
+  @Test
--- End diff --

@ottobackwards , how about "Mocking sucks"? :-)
Or at least "Mocking complex objects is excruciating."
But seriously, yes I'll add some comments.  Not planning on a line-by-line 
walk-thru, tho.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread mattf-horton
Github user mattf-horton commented on a diff in the pull request:

https://github.com/apache/metron/pull/481#discussion_r127076248
  
--- Diff: 
metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/writer/SingleBatchConfigurationFacade.java
 ---
@@ -18,6 +18,8 @@
 
 package org.apache.metron.common.configuration.writer;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 
--- End diff --

Well, there's some weirdness in this stuff, and I didn't try to get rid of 
it lest I break something I didn't understand.  In this case (and other 
places), it appears that non-batched writers are essentially faked by using 
batched writers with a batchSize of "1".  Doesn't seem real efficient to me, 
but obviously decreases the overall quantity of code and complexity of testing. 
 Anyway, since I only did a trivial change to this file, I didn't feel 
obligated to try to explain it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[REQUEST] Contributor rights in Jira

2017-07-12 Thread Laurens Vets

Hello,

Could a PMC member please grant my Jira account contributor rights? I'd 
like to start helping out with various smaller tasks. I promise I won't 
mess stuff up and go to IRC first for any questions/comments/additions. 
Otto has been extremely helpful there already :)


Thanks,
Laurens


[GitHub] metron issue #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread mattf-horton
Github user mattf-horton commented on the issue:

https://github.com/apache/metron/pull/481
  
Comment on testing:  There are so many permutations it only seemed 
reasonable to automate them in unit test, and so I did.  As part of code 
review, please provide your opinion on whether the provided unit tests are 
adequate, or what additional test cases should be added.

Manual end-to-end testing, if you are so moved, consists of six scenarios 
for a given sensor queue:
1. Under **heavy continuous load** the batchSize still controls flushing 
behavior, because the queue size always exceeds batchSize before queue age 
exceeds batchTimeout.
2. Under **light continuous load**, where each queue continues to receive 
at least one message per second, and batchSize is large enough it is never 
exceeded, then the batchTimeout for each queue should control flushing behavior 
within +/- 1 sec, because each new message triggers a check of the queue age 
and potential timeout flush.
  - NOTE: If the configured batchTimeout is set to a large number, bigger 
than `1/2 topology.message.timeout.secs - 1` (which equals **14 sec** by 
default), then it will be replaced by an effective value equal to `1/2 
topology.message.timeout.secs - 1`.  Flushing will occur within +/- 1 sec of 
each _effective_ batchTimeout interval, rather than the _configured_ 
batchTimeout interval.
3. Under **light intermittent load**, where less than batchSize messages 
queue up, and gaps between messages may exceed the timeout interval, then age 
checks and potential flush events may be triggered by _either_ incoming 
messages or TickTuple events, depending on the phase relationship between 
intermittent bursts of messages, and the TickTuple system tick.  The TickTuple 
interval is guaranteed to be < `1/2 topology.message.timeout.secs`, hence the 
default TickTuple interval is 14 seconds.  But if the smallest batchTimeout 
configured for any sensor queue on the Bolt is < the default TickTuple 
interval, then that smallest value becomes the actual TickTuple interval.  This 
produces three sub-cases, all of which guarantee a flush event before any 
message gets recycled due to aging past `topology.message.timeout.secs`:
  - If the queue's configured batchTimeout is the smallest (or only) such 
on this Bolt, and that number is smaller than the default TickTuple interval, 
then it _becomes_ the actual TickTuple interval.  The queue is guaranteed to 
flush between 1x and 2x this interval.
  - If the queue's configured batchTimeout is not the smallest such, but 
still is < the default TickTuple interval, then the queue is guaranteed to 
flush between its own `configured batchTimeout` and its `configured 
batchTimeout + actual TickTuple interval` (which is less than 2x its own 
`configured batchTimeout`).
  - If the queue's configured batchTimeout is > the default TickTuple 
interval (14 sec default), then its effective batchTimeout is set to the 
default TickTuple interval.  The queue is guaranteed to flush between this 
`effective batchTimeout` and its `effective batchTimeout + actual TickTuple 
interval`.

The upshot is that:
* "Configured batchTimeout" should be thought of as "minimum age before 
you'll allow a time-based flush" (capped by default TickTuple interval, aka 1/2 
`topology.message.timeout.secs`)
* "Actual TickTuple interval" is the "maximum time between age checks".  It 
will be <= all the configured batchTimeouts for the various sensors on the Bolt.
* When a flush actually happens may be up to "effective batchTimeout" + 
"actual TickTuple interval", depending on exactly when intermittent message 
events and periodic Tick events happen.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[REQUEST] Wiki Edit Rights

2017-07-12 Thread Kyle Richardson
Could a PMC member please grant my account edit rights to the Metron space
in Confluence? I'd like to start helping with the clean up efforts there.

Thanks,
Kyle


[GitHub] metron issue #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread ottobackwards
Github user ottobackwards commented on the issue:

https://github.com/apache/metron/pull/481
  
I am seeing errors like
```bash

---
 T E S T S
---
Running org.apache.metron.solr.integration.SolrIndexingIntegrationTest
Waiting for zookeeper...
Found index config in zookeeper...
0 vs 10 vs 0
0 vs 10 vs 0
0 vs 10 vs 0
0 vs 10 vs 0
0 vs 10 vs 0
0 vs 10 vs 0
Processed 
target/indexingIntegrationTest/hdfs/test/enrichment-hdfsIndexingBolt-1-0-1499888936956.json
10 vs 10 vs 10
Processed 
target/indexingIntegrationTest/hdfs/test/enrichment-hdfsIndexingBolt-1-0-1499888936956.json
2017-07-12 15:48:57 ERROR ClientCnxn:524 - Error while calling watcher
java.util.concurrent.RejectedExecutionException: Task 
org.apache.solr.common.util.ExecutorUtil$MDCAwareThreadPoolExecutor$1@9651f04 
rejected from 
org.apache.solr.common.util.ExecutorUtil$MDCAwareThreadPoolExecutor@d5a9af0[Terminated,
 pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 16]
at 
java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2047)
at 
java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:823)
at 
java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1369)
at 
org.apache.solr.common.util.ExecutorUtil$MDCAwareThreadPoolExecutor.execute(ExecutorUtil.java:135)
at 
java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:112)
at 
org.apache.solr.common.cloud.SolrZkClient$3.process(SolrZkClient.java:261)
at 
org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:522)
at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:498)
2017-07-12 15:49:01 ERROR ReadClusterState:181 - Failed to Sync Supervisor
java.lang.RuntimeException: java.lang.InterruptedException
at org.apache.storm.utils.Utils.wrapInRuntime(Utils.java:1507)
at org.apache.storm.zookeeper.Zookeeper.getChildren(Zookeeper.java:260)
at 
org.apache.storm.cluster.ZKStateStorage.get_children(ZKStateStorage.java:174)
at 
org.apache.storm.cluster.StormClusterStateImpl.assignments(StormClusterStateImpl.java:153)
at 
org.apache.storm.daemon.supervisor.ReadClusterState.run(ReadClusterState.java:126)
at org.apache.storm.event.EventManagerImp$1.run(EventManagerImp.java:54)
Caused by: java.lang.InterruptedException
at java.lang.Object.wait(Native Method)
at java.lang.Object.wait(Object.java:502)
at 
org.apache.storm.shade.org.apache.zookeeper.ClientCnxn.submitRequest(ClientCnxn.java:1342)
at 
org.apache.storm.shade.org.apache.zookeeper.ZooKeeper.getChildren(ZooKeeper.java:1588)
at 
org.apache.storm.shade.org.apache.zookeeper.ZooKeeper.getChildren(ZooKeeper.java:1625)
at 
org.apache.storm.shade.org.apache.curator.framework.imps.GetChildrenBuilderImpl$3.call(GetChildrenBuilderImpl.java:210)
at 
org.apache.storm.shade.org.apache.curator.framework.imps.GetChildrenBuilderImpl$3.call(GetChildrenBuilderImpl.java:203)
at 
org.apache.storm.shade.org.apache.curator.RetryLoop.callWithRetry(RetryLoop.java:108)
at 
org.apache.storm.shade.org.apache.curator.framework.imps.GetChildrenBuilderImpl.pathInForeground(GetChildrenBuilderImpl.java:200)
at 
org.apache.storm.shade.org.apache.curator.framework.imps.GetChildrenBuilderImpl.forPath(GetChildrenBuilderImpl.java:191)
at 
org.apache.storm.shade.org.apache.curator.framework.imps.GetChildrenBuilderImpl.forPath(GetChildrenBuilderImpl.java:38)
at org.apache.storm.zookeeper.Zookeeper.getChildren(Zookeeper.java:255)
... 4 more
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 45.675 sec 
- in org.apache.metron.solr.integration.SolrIndexingIntegrationTest

Results :
```
Have you seen these?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron issue #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread ottobackwards
Github user ottobackwards commented on the issue:

https://github.com/apache/metron/pull/481
  
This is a lot of work, and a great job @mattf-horton.  A couple of nit 
comments I would like to get response to before a +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread ottobackwards
Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/481#discussion_r127053228
  
--- Diff: 
metron-platform/metron-writer/src/main/java/org/apache/metron/writer/bolt/BulkMessageWriterBolt.java
 ---
@@ -92,24 +177,58 @@ public void prepare(Map stormConf, TopologyContext 
context, OutputCollector coll
   configurationTransformation = x -> x;
 }
 try {
-  bulkMessageWriter.init(stormConf,
- context,
- configurationTransformation.apply(new 
IndexingWriterConfiguration(bulkMessageWriter.getName(),
- getConfigurations()))
-);
+  WriterConfiguration writerconf = configurationTransformation.apply(
+  new IndexingWriterConfiguration(bulkMessageWriter.getName(), 
getConfigurations()));
+  if (defaultBatchTimeout == 0) {
+//This means getComponentConfiguration was never called to 
initialize defaultBatchTimeout,
+//probably because we are in a unit test scenario.  So calculate 
it here.
+BatchTimeoutHelper timeoutHelper = new 
BatchTimeoutHelper(writerconf::getAllConfiguredTimeouts, batchTimeoutDivisor);
+defaultBatchTimeout = timeoutHelper.getDefaultBatchTimeout();
+  }
+  writerComponent.setDefaultBatchTimeout(defaultBatchTimeout);
+  bulkMessageWriter.init(stormConf, context, writerconf);
 } catch (Exception e) {
   throw new RuntimeException(e);
 }
   }
 
+  /**
+   * Used only for unit testing.
+   */
--- End diff --

Maybe we can name this testPrepare? So it is explicit to caller, who may 
not 'peek' inside the source


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread ottobackwards
Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/481#discussion_r127052398
  
--- Diff: 
metron-platform/metron-enrichment/src/test/java/org/apache/metron/enrichment/bolt/BulkMessageWriterBoltTest.java
 ---
@@ -164,4 +174,100 @@ public void test() throws Exception {
 verify(outputCollector, times(1)).emit(eq(Constants.ERROR_STREAM), 
any(Values.class));
 verify(outputCollector, times(1)).reportError(any(Throwable.class));
   }
+
+  @Test
--- End diff --

Maintainers will thank you for throwing some comments here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing

2017-07-12 Thread ottobackwards
Github user ottobackwards commented on a diff in the pull request:

https://github.com/apache/metron/pull/481#discussion_r127051759
  
--- Diff: 
metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/writer/SingleBatchConfigurationFacade.java
 ---
@@ -18,6 +18,8 @@
 
 package org.apache.metron.common.configuration.writer;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 
--- End diff --

It would be nice if there was some javadoc on this class as to it's purpose 
and usage


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


code review: adding time-based flushing to BulkMessageWriterBolt

2017-07-12 Thread Matt Foley
I think PR#481, for the first two-thirds of METRON322 (sub-tasks 2, 3, 4, and 
7), is ready for review and inclusion.  If folks have time to review it would 
be appreciated.

 

Thanks,

--Matt

 



[GitHub] metron pull request #648: METRON-1033 Corrected profiler docs units on expir...

2017-07-12 Thread simonellistonball
GitHub user simonellistonball opened a pull request:

https://github.com/apache/metron/pull/648

METRON-1033 Corrected profiler docs units on expires field

Minor change to update profiler docs

## Contributor Comments
[Please place any comments here.  A description of the problem/enhancement, 
how to reproduce the issue, your testing methodology, etc.]


## 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 documentation related changes:
- [x] 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`:


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/simonellistonball/incubator-metron METRON-1033

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/metron/pull/648.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 #648


commit 880e10cfc31f49cb11a3f5639c2c0a217743c044
Author: Simon Elliston Ball 
Date:   2017-07-12T16:30:25Z

Corrected profiler docs units on expires field




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #631: METRON-1019: Metron 0.4.0 manual installation guid...

2017-07-12 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/metron/pull/631


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #639: METRON-1013 add command line verification to stell...

2017-07-12 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/metron/pull/639


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #647: METRON-1031: Management UI Cannot Start Topologies...

2017-07-12 Thread nickwallen
GitHub user nickwallen opened a pull request:

https://github.com/apache/metron/pull/647

METRON-1031: Management UI Cannot Start Topologies in Kerberized Environment

The Metron Management UI cannot start topologies in a kerberized 
environment.  This includes the Parser, Enrichment, and Indexing topologies.

It seems that Metron REST does not pass "-ksp" to the start topology 
commands. As a result, topologies that are started with the Management UI 
cannot start a producer against a kerberized Kafka.

### Changes

* Altered the `StormCLIWrapper` to add logging of the actual CLI commands 
that are executed.
* The `-ksp` argument is always used (with Kerberos or not) when launching 
the Parser, Enrichment, and Indexing topologies.
* Added a Spring property `kafka.security.protocol` that serves as the 
`-ksp` argument when launching the topologies.
* Ambari sets this Spring property based on Kafka's own configuration in 
Ambari.

### Testing

To manually test this, validate that all of the topologies can be stopped 
and started using the Management UI and Swagger UI both before and after 
kerberization.

1. Launch the Full Dev environment.
1. Stop then start the Parser topologies using the Management UI.
1. Stop then start the Enrichment and Indexing topologies using the Swagger 
UI.
1. Kerberize the Full Dev environment.
1. Stop then start the Parser topologies using the Management UI.
1. Stop then start the Enrichment and Indexing topologies using the Swagger 
UI.

### Pull Request Checklist

- [ ] 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)?
- [ ] 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:
- [ ] 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)? 
- [ ] Have you verified the basic functionality of the build by building 
and running locally with Vagrant full-dev environment or the equivalent?



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/nickwallen/metron METRON-1031

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/metron/pull/647.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 #647


commit 6cff444a1120ca356aa4e39f54981f2d43b5
Author: Nick Allen 
Date:   2017-07-11T23:05:26Z

METRON-1031: Management UI Cannot Start Topologies in Kerberized Environment

commit a70ad836f9a08e58dc97c0cfc4d5dcaabf5c818b
Author: Nick Allen 
Date:   2017-07-12T13:52:04Z

Configuration fixup




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...

2017-07-12 Thread iraghumitra
Github user iraghumitra commented on a diff in the pull request:

https://github.com/apache/metron/pull/620#discussion_r126957989
  
--- Diff: metron-interface/metron-alerts/src/app/service/data-source.ts ---
@@ -0,0 +1,62 @@
+/**
+ * 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 {Observable} from 'rxjs/Rx';
+import {Injectable} from '@angular/core';
+import {Http} from '@angular/http';
+
+import {Alert} from '../model/alert';
+import {ColumnMetadata} from '../model/column-metadata';
+import {ColumnNames} from '../model/column-names';
+import {TableMetadata} from '../model/table-metadata';
+import {SaveSearch} from '../model/save-search';
+import {AlertsSearchResponse} from '../model/alerts-search-response';
+import {SearchRequest} from '../model/search-request';
+
+@Injectable()
+export abstract class DataSource {
+  defaultHeaders: {'Content-Type': 'application/json', 'X-Requested-With': 
'XMLHttpRequest'};
+
+  constructor(protected http: Http) {}
+
+  // Calls to fetch alerts
+  abstract getAlerts(searchRequest: SearchRequest): 
Observable
+  abstract getAlert(index: string, type: string, alertId: string): 
Observable
+  abstract updateAlertState(request: any): Observable<{}>
+
+  // Calls to fetch default alert table column names and all the field 
names across all indexes
+  abstract getDefaultAlertTableColumnNames(): Observable
+  abstract getAllFieldNames(): Observable
+
+  // Calls to rename field names and to fetch the renamed field names
+  abstract getAlertTableColumnNames(): Observable
+  abstract saveAlertTableColumnNames(columns: ColumnNames[]): 
Observable<{}>
+
+  // Calls to fetch and save alerts table settings like refresh interval, 
page size, default selected table column names
+  abstract getAlertTableSettings(): Observable
--- End diff --

The idea here is to provide an abstraction for all the API's not just the 
ES API's. 

In theory, if we want GUI to use rest API instead of directly querying ES 
and storing data in the local browser cache the API provider should support all 
the API's defined in data-source.ts. 
The API's can be written in phases we would create a separate Impl class 
that extends Datasource and use the API's that are available. 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron issue #620: Metron-988: UI for viewing alerts generated by Metron

2017-07-12 Thread mraliagha
Github user mraliagha commented on the issue:

https://github.com/apache/metron/pull/620
  
@iraghumitra We are using ASA and CEF parsers. Can't you get the field 
names dynamically from Elasticsearch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron issue #639: METRON-1013 add command line verification to stellar shel...

2017-07-12 Thread cestella
Github user cestella commented on the issue:

https://github.com/apache/metron/pull/639
  
+1 by inspection


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #634: METRON-1017: Ambari components should be separate

2017-07-12 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/metron/pull/634


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...

2017-07-12 Thread iraghumitra
Github user iraghumitra commented on a diff in the pull request:

https://github.com/apache/metron/pull/620#discussion_r126949623
  
--- Diff: metron-interface/metron-alerts/src/app/model/alert.ts ---
@@ -0,0 +1,44 @@
+export class Alert {
--- End diff --

I agree the name alert might be a misnomer the accurate name should reflect 
that this class holds all the data in the ES indexes.  Any suggestions?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron issue #620: Metron-988: UI for viewing alerts generated by Metron

2017-07-12 Thread iraghumitra
Github user iraghumitra commented on the issue:

https://github.com/apache/metron/pull/620
  
@mraliagha I guess I found the issue. I was fetching the column names only 
from bro indexes. I fixed it in the latest commit. Do you have bro indexes ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...

2017-07-12 Thread iraghumitra
Github user iraghumitra commented on a diff in the pull request:

https://github.com/apache/metron/pull/620#discussion_r126948453
  
--- Diff: 
metron-interface/metron-alerts/src/app/utils/elasticsearch-utils.ts ---
@@ -0,0 +1,72 @@
+/**
+ * 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 {ColumnMetadata} from '../model/column-metadata';
+import {AlertsSearchResponse} from '../model/alerts-search-response';
+
+export class ElasticsearchUtils {
+
+  private static createColumMetaData(properties: any, columnMetadata: 
ColumnMetadata[], seen: string[]) {
+ try {
+   let columnNames = Object.keys(properties);
+   for (let columnName of columnNames) {
+ if (seen.indexOf(columnName) === -1) {
+   seen.push(columnName);
+   columnMetadata.push(
+ new ColumnMetadata(columnName, (properties[columnName].type ? 
properties[columnName].type.toUpperCase() : ''))
+   );
+ }
+   }
+ } catch (e) {}
+  }
+
+  public static extractColumnNameData(res: Response): ColumnMetadata[] {
+let response: any = res || {};
+let columnMetadata: ColumnMetadata[] = [];
+let seen: string[] = [];
+
+for (let index in response.metadata.indices) {
+  if (index.startsWith('bro') || index.startsWith('bro') || 
index.startsWith('bro')) {
--- End diff --

Yup, good catch replaced the condition with much better one. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...

2017-07-12 Thread iraghumitra
Github user iraghumitra commented on a diff in the pull request:

https://github.com/apache/metron/pull/620#discussion_r126948282
  
--- Diff: 
metron-interface/metron-alerts/src/app/service/elasticsearch-localstorage-impl.ts
 ---
@@ -0,0 +1,291 @@
+/**
+ * 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 {Observable} from 'rxjs/Rx';
+import {Headers, RequestOptions} from '@angular/http';
+
+import {HttpUtil} from '../utils/httpUtil';
+import {DataSource} from './data-source';
+import {Alert} from '../model/alert';
+import {ColumnMetadata} from '../model/column-metadata';
+import {ElasticsearchUtils} from '../utils/elasticsearch-utils';
+import {
+  ALERTS_COLUMN_NAMES, ALERTS_TABLE_METADATA, ALERTS_RECENT_SEARCH,
+  ALERTS_SAVED_SEARCH, NUM_SAVED_SEARCH
+} from '../utils/constants';
+import {ColumnNames} from '../model/column-names';
+import {ColumnNamesService} from './column-names.service';
+import {TableMetadata} from '../model/table-metadata';
+import {SaveSearch} from '../model/save-search';
+import {AlertsSearchResponse} from '../model/alerts-search-response';
+import {SearchRequest} from '../model/search-request';
+
+export class ElasticSearchLocalstorageImpl extends DataSource {
+
+  private defaultColumnMetadata = [
+new ColumnMetadata('_id', 'string'),
+new ColumnMetadata('timestamp', 'date'),
+new ColumnMetadata('source:type', 'string'),
+new ColumnMetadata('ip_src_addr', 'ip'),
+new ColumnMetadata('enrichments:geo:ip_dst_addr:country', 'string'),
+new ColumnMetadata('ip_dst_addr', 'ip'),
+new ColumnMetadata('host', 'string'),
+new ColumnMetadata('alert_status', 'string')
+  ];
+
+  getAlerts(searchRequest: SearchRequest): 
Observable {
+let url = '/search/*,-*kibana/_search';
+return this.http.post(url, searchRequest, new RequestOptions({headers: 
new Headers(this.defaultHeaders)}))
--- End diff --

Fixed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...

2017-07-12 Thread iraghumitra
Github user iraghumitra commented on a diff in the pull request:

https://github.com/apache/metron/pull/620#discussion_r126948073
  
--- Diff: metron-interface/metron-alerts/src/app/model/search-request.ts ---
@@ -0,0 +1,7 @@
+export class SearchRequest {
+  _source: string[];
+  query = { query_string: { query: '' } };
--- End diff --

Fixed the type to string


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...

2017-07-12 Thread iraghumitra
Github user iraghumitra commented on a diff in the pull request:

https://github.com/apache/metron/pull/620#discussion_r126947986
  
--- Diff: 
metron-interface/metron-alerts/src/app/alerts/saved-searches/saved-searches.component.ts
 ---
@@ -0,0 +1,124 @@
+import { Component, OnInit } from '@angular/core';
+import {Router} from '@angular/router';
+import {Observable} from 'rxjs/Rx';
+
+import {SaveSearchService} from '../../service/save-search.service';
+import {SaveSearch} from '../../model/save-search';
+import {MetronDialogBox} from '../../shared/metron-dialog-box';
+import {NUM_SAVED_SEARCH} from '../../utils/constants';
+
+@Component({
+  selector: 'app-saved-searches',
+  templateUrl: './saved-searches.component.html',
+  styleUrls: ['./saved-searches.component.scss']
+})
+export class SavedSearchesComponent implements OnInit {
+
+  searches: SaveSearch[];
+  recentSearcheObj: SaveSearch[];
+  savedSearches: any = {};
--- End diff --

There was a separate model to pass data to collapse component the type 
declaration for it was missing here. Since i was looking at the code after a 
while i got a chance to improve now :). The code looks much better now. In gist 
I fixed this 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...

2017-07-12 Thread iraghumitra
Github user iraghumitra commented on a diff in the pull request:

https://github.com/apache/metron/pull/620#discussion_r126947375
  
--- Diff: 
metron-interface/metron-alerts/src/app/alerts/alerts-list/query-builder.ts ---
@@ -0,0 +1,139 @@
+/**
+ * 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 {Filter} from '../../model/filter';
+import {ColumnNamesService} from '../../service/column-names.service';
+import {SearchRequest} from '../../model/search-request';
+
+export class QueryBuilder {
+  private _searchRequest = new SearchRequest();
+  private _query = '*';
+  private _displayQuery = this._query;
+  private _filters: Filter[] = [];
+
+  set query(value: string) {
+value = value.replace(/\\:/g, ':');
+this._query = value;
+this.updateFilters(this._query, false);
+this.onSearchChange();
+  }
+
+  get query(): string {
+return this._query;
+  }
+
+  set displayQuery(value: string) {
+this._displayQuery = value;
+this.updateFilters(this._displayQuery, true);
+this.onSearchChange();
+  }
+
+  get displayQuery(): string {
+return this._displayQuery;
+  }
+
+  get filters(): Filter[] {
+return this._filters;
+  }
+
+
+  get searchRequest(): SearchRequest {
+this._searchRequest.query = { query_string: { query: 
this.generateSelect() } };
+return this._searchRequest;
+  }
+
+  set searchRequest(value: SearchRequest) {
+this._searchRequest = value;
+this.query = this._searchRequest.query.query_string.query;
--- End diff --

Fixed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...

2017-07-12 Thread iraghumitra
Github user iraghumitra commented on a diff in the pull request:

https://github.com/apache/metron/pull/620#discussion_r126947344
  
--- Diff: 
metron-interface/metron-alerts/src/app/alerts/alerts-list/query-builder.ts ---
@@ -0,0 +1,139 @@
+/**
+ * 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 {Filter} from '../../model/filter';
+import {ColumnNamesService} from '../../service/column-names.service';
+import {SearchRequest} from '../../model/search-request';
+
+export class QueryBuilder {
+  private _searchRequest = new SearchRequest();
+  private _query = '*';
+  private _displayQuery = this._query;
+  private _filters: Filter[] = [];
+
+  set query(value: string) {
+value = value.replace(/\\:/g, ':');
+this._query = value;
+this.updateFilters(this._query, false);
+this.onSearchChange();
+  }
+
+  get query(): string {
+return this._query;
+  }
+
+  set displayQuery(value: string) {
+this._displayQuery = value;
+this.updateFilters(this._displayQuery, true);
+this.onSearchChange();
+  }
+
+  get displayQuery(): string {
+return this._displayQuery;
+  }
+
+  get filters(): Filter[] {
+return this._filters;
+  }
+
+
+  get searchRequest(): SearchRequest {
+this._searchRequest.query = { query_string: { query: 
this.generateSelect() } };
--- End diff --

Fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron issue #636: METRON-1022: Elasticsearch REST endpoint

2017-07-12 Thread cestella
Github user cestella commented on the issue:

https://github.com/apache/metron/pull/636
  
@ottobackwards agreed, we should work to that goal.  I think we might want 
to make baby steps, though, and create the abstractions first and move to 
making it pluggable later.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron issue #636: METRON-1022: Elasticsearch REST endpoint

2017-07-12 Thread ottobackwards
Github user ottobackwards commented on the issue:

https://github.com/apache/metron/pull/636
  
@cestella 
https://issues.apache.org/jira/browse/METRON-956
Is the jira I had created on this topic.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron pull request #641: METRON-539: added HASH function for stellar.

2017-07-12 Thread jjmeyer0
Github user jjmeyer0 commented on a diff in the pull request:

https://github.com/apache/metron/pull/641#discussion_r126926090
  
--- Diff: 
metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/dsl/functions/HashFunctionsTest.java
 ---
@@ -0,0 +1,169 @@
+/*
+ * 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.
+ */
+package org.apache.metron.stellar.dsl.functions;
+
+import com.google.common.io.BaseEncoding;
+import org.apache.commons.lang.SerializationUtils;
+import org.junit.Test;
+
+import java.io.Serializable;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Security;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import static 
org.apache.metron.stellar.common.utils.StellarProcessorUtils.run;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+public class HashFunctionsTest {
+  final HashFunctions.Hash hash = new HashFunctions.Hash();
+
+  @Test(expected = IllegalArgumentException.class)
+  public void nullArgumentListShouldThrowException() throws Exception {
+hash.apply(null);
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void emptyArgumentListShouldThrowException() throws Exception {
+hash.apply(Collections.emptyList());
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void singleArgumentListShouldThrowException() throws Exception {
+hash.apply(Collections.singletonList("some value."));
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void argumentListWithMoreThanTwoValuesShouldThrowException3() 
throws Exception {
+hash.apply(Arrays.asList("1", "2", "3"));
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void argumentListWithMoreThanTwoValuesShouldThrowException4() 
throws Exception {
+hash.apply(Arrays.asList("1", "2", "3", "4"));
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void invalidAlgorithmArgumentShouldThrowException() throws 
Exception {
+hash.apply(Arrays.asList("value to hash", "invalidAlgorithm"));
+  }
+
+  @Test
+  public void invalidNullAlgorithmArgumentShouldThrowException() throws 
Exception {
+assertNull(hash.apply(Arrays.asList("value to hash", null)));
+  }
+
+  @Test
+  public void nullInputForValueToHashShouldProperlyThrowException() throws 
Exception {
+assertNull(hash.apply(Arrays.asList(null, "md5")));
+  }
--- End diff --

oh boy, @mattf-horton that makes much more sense. forgive me i had a long 
day yesterday. I had a moment haha


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron issue #636: METRON-1022: Elasticsearch REST endpoint

2017-07-12 Thread cestella
Github user cestella commented on the issue:

https://github.com/apache/metron/pull/636
  
I just want to follow-up with something a bit more concrete suggestions.  I 
think the beginnings of an abstraction are there.  You pulled out a bunch of 
utility methods from `ElasticsearchWriter` which is good.  I'm looking for a 
DAO-like abstraction in `metron-elasticsearch` and the `ServiceImpl` in the 
REST layer uses the generic DAO implementation (which would not live in the 
REST layer so it's usable for lower level access) and the index impl is 
specified by at runtime.  The impl for the DAO would live in one of 
`metron-elasticsearch` or `metron-solr`.

Again, these are my $0.02 and just suggestions.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] metron issue #636: METRON-1022: Elasticsearch REST endpoint

2017-07-12 Thread cestella
Github user cestella commented on the issue:

https://github.com/apache/metron/pull/636
  
Looking at this implementation and working a bit on the PoC for index data 
mutation, I think the abstraction here isn't in the right place.  It's too 
bound-up in the REST layer whereas data access has traditionally existed 
further down in the abstraction layer.  It's a smell that metron-rest has to 
depend on ES at all.  That dependency should be provided at runtime entirely.  
Also, we're going to need data access in batch as well and I'd like a sensible 
abstraction in place to enable that (e.g. merging modified records when reading 
as part of a Spark or MR job).

It seems to me that we need a DAO abstraction living in `metron-writer` or 
even `metron-indexing` for interacting with indices with the concrete 
implementations existing in `metron-elasticsearch` and `metron-solr` for ES and 
Solr respectively.  I think `metron-rest` should work entirely on abstract 
classes.  The REST layer should choose the DAO implementation via reflection 
and we should adjust the classpath in the shell script to include one of 
`metron-elasticsearch` or `metron-solr`.  In essence, I'm recommending 
something similar to how JDBC works where you choose your driver implementation 
via reflection (often) and work with base JDBC classes from there.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---