[GitHub] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
All integration tests are passing.  I was able to start up the application 
against quick dev and all the endpoints look like they are working.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
For convenience, I staged the commit in a remote branch: 
https://github.com/cestella/incubator-metron/tree/METRON-503_sandbox .  I'd ask 
that @merrimanr take it for a test drive.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr @jjmeyer0 I think we should make sure JJ gets credit for his 
contribution on this.  We should squash the commits into the minimal possible 
each named "METRON-503: Metron REST API this closes 
apache/incubator-metron#316" with the appropriate authorship.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Ok, this is a big one and has been around for a long time now in 
Metron-years.  Before we go ahead and pull the trigger on commit, I want to 
make sure there are no outstanding comments that need to be addressed.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
First off, I'd like to apologize for it taking me a little longer to retest 
this.

I went through and tested the issues I saw again. Everything seems to be 
working. I think this is ready to go. It's a great feature. Thanks for 
implementing it!

Also, since this is a big PR, don't worry about giving me credit on my 
commits. Especially if it makes your life easier when squashing everything. If 
that's allowed anyway. 

+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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr Good catch on this. I'll take a look again, but I think it's 
looking good!


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Finally figured this one out.  I wasn't able to reproduce the issue because 
my maven version was a couple minor releases behind (on 3.3.9 now for what it's 
worth).  Once I got past that the problem was obvious:  metron-common was 
bringing in shaded org.reflections classes directly and regular org.reflections 
classes transitively which are not compatible because guava is rewritten in the 
shaded classes, hence the NoSuchMethodError.  So for now the fix is to exclude 
org.reflections from the metron-common dependency and only depend on the shaded 
version.

I had to merge several commits in so I'm still testing for regressions but 
this issue should be resolved.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr I had a chance to try this again. If I build from 
`incubator-metron` it doesn't work. If I build from `metron-interface` 
everything seems to be working just fine.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Hmm I just tried deploying on quick-dev the same way and was able to hit 
the stellar endpoints without issue.  I will continue testing to see if I can 
get it to break tomorrow.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr I'm using quick-dev to do my tests. I built everything from 
metron root with `mvn clean install -DskipTests`. Then I setup `quick-dev`. 
Extracted the tar and ran the following:

```
java -jar ./lib/metron-rest-$METRON_VERSION.jar 
--spring.profiles.active=vagrant,dev --server.port=8082
```


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Just merged in the latest from master including the maxmind fix that was 
breaking the build.

I am not able to recreate the NoSuchError exception.  Here is what I'm 
doing to test:
//from project root
mvn clean install -DskipTests
java -jar ./metron-interface/metron-rest/target/metron-rest-0.3.1.jar 
--spring.profiles.active=docker,dev

When I hit the stellar endpoints, the functions come back without error.  
How are you testing 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.
---

[GitHub] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr For some reason I am still getting the following error on the 
endpoints. Also, the no such method error turned into this as well. How are you 
running it? I'm running it using java cli. Are you using IntelliJ or something 
else?

```
Feb 23, 2017 12:51:24 PM org.apache.catalina.core.StandardWrapperValve 
invoke
SEVERE: Servlet.service() for servlet [dispatcherServlet] in context with 
path [] threw exception [Handler dispatch failed; nested exception is 
java.lang.NoSuchMethodError: 
org.reflections.util.ConfigurationBuilder.filterInputsBy(Lorg/apache/metron/guava/base/Predicate;)Lorg/reflections/util/ConfigurationBuilder;]
 with root cause
java.lang.NoSuchMethodError: 
org.reflections.util.ConfigurationBuilder.filterInputsBy(Lorg/apache/metron/guava/base/Predicate;)Lorg/reflections/util/ConfigurationBuilder;
at 
org.apache.metron.common.dsl.functions.resolver.ClasspathFunctionResolver.resolvables(ClasspathFunctionResolver.java:167)
at 
org.apache.metron.common.dsl.functions.resolver.BaseFunctionResolver.resolveFunctions(BaseFunctionResolver.java:119)
at 
org.apache.metron.guava.base.Suppliers$MemoizingSupplier.get(Suppliers.java:125)
at 
org.apache.metron.common.dsl.functions.resolver.BaseFunctionResolver.getFunctionInfo(BaseFunctionResolver.java:77)
at 
org.apache.metron.rest.service.impl.StellarServiceImpl.getStellarFunctions(StellarServiceImpl.java:74)
at 
org.apache.metron.rest.controller.StellarController.listFunctions(StellarController.java:71)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at 
org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:221)
at 
org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:136)
at 
org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:114)
at 
org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:827)
at 
org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:738)
at 
org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:85)
at 
org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:963)
at 
org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:897)
at 
org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:970)
at 
org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:861)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:622)
at 
org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:846)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:729)
at 
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:230)
at 
org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:165)
at 
org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
at 
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:192)
at 
org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:165)
at 
org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:317)
at 
org.springframework.security.web.access.intercept.FilterSecurityInterceptor.invoke(FilterSecurityInterceptor.java:127)
at 
org.springframework.security.web.access.intercept.FilterSecurityInterceptor.doFilter(FilterSecurityInterceptor.java:91)
at 
org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
at 
org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:115)
at 
org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
at 
org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:137)
at 

[GitHub] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Just pushed out commits to address these bugs.  In regards to this:  

"Another thing that may be nice is to describe what is all needed for the 
REST API to run. For example, I believe it needs access to storm cli, some 
scripts, and some other things."

This is included at the top of the metron-rest README in the Prerequisites 
section.  It's changed a couple times so you may have just missed it.  If there 
are other dependencies you think we should call out, let me know.

Thanks for the feedback.  I think we're almost 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.
---

[GitHub] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
The latest commit includes removing MySQL as the security credential store 
and replacing with H2.  I also updated the documentation with instructions on 
configuring a production database (like MySQL) for this purpose.

Unit and integration tests are passing.  I tested this on both the 
metron-docker and Quick Dev environment.  This can be tested on metron-docker 
with the spring boot maven plugin and docker profile:

//navigate to metron-rest
mvn spring-boot:run -Drun.profiles=docker,dev

I also added a vagrant profile for testing on Quick Dev.  To test this on 
Quick Dev, scp the metron-rest archive to node1 and extract the archive.   Then 
start the application with the vagrant profile and change the port so that it 
doesn't conflict with Ambari:

java -jar ./lib/metron-rest-0.3.0.jar --spring.profiles.active=vagrant,dev 
--server.port=8082

I did a fairly comprehensive tests and all endpoints are working.  Once we 
are satisfied with this I will take another pass and fix the minor style issues 
(spacing, adherence to checkstyle, etc).


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr sorry for not commenting earlier. I think this is in a pretty 
good state. I do want to review this again from beginning to end after you 
finish the MySQL stuff. After that I'll either +1 or add some comments. It's 
looking good though!


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr the builds failing due to the `Cli to CLI` change. I've hit this 
before. I think it's because mac is case insensitive and gets confused. To fix 
it, I've had to manually change the file name in github.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr you are right about guava. I'll remove my uses of it. Plus, Java 
has a lot of those functions built in now. No reason I shouldn't use those 
instead.

I agree that the guava issue should be taken care of separately. Right now, 
I think we should allow a user to set an envitonment variable to point to which 
guava to use (similar to hibernate and mysql). This will make it easier for us 
to run it outside of our IDE.

I think your changes look pretty good. Really nice clean up/tests. We've 
changed a lot, so I want to do another pass though.

I think we should still have our `HdfsService`. Right now, I think using 
`FileSystem` directly works for us. Using Knox is a good idea, but it probably 
needs to be optional. My thought is having the service will help us achieve 
that. You are probably right though. If we can achieve everything with WebHDFS 
it's better for our service to use that. This PR is already really big though. 
Maybe we can have that as a take away?



---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
It just occurred to me that we don't have an HdfsController.  Do we even 
need an HdfsService or could we just use WebHDFS (or Knox) instead?


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@jjmeyer0, merged your PR in with the new tests.  Everything looked good, 
thanks for helping with that.

I just submitted another commit with tests for the remaining services 
excluding TransformationValidationService (still thinking about how to rename 
that).  Will have that one done soon.  I also refactored the 
SensorParserConfigServiceImpl by removing all the Grok functions with the 
exception of the isGrokParser method.  The read/write methods for Grok 
statements are exactly the same as the read/write methods in the 
HdfsServiceImpl class and are not needed so I removed them.  I feel like the 
saveTemporary method should be in GrokServiceImpl so I moved it there.  With 
this change, SensorParserConfigServiceImpl more closely matches the other CRUD 
interfaces.  Let me know what you think about that.

As for the error you ran into, I have hit that as well many times while 
working on Metron.  The underlying problem is that the shade plugin is not 
correctly configured on the metron-statistics module.  The guava library is not 
shaded and the uber jar is installed instead of the regular jar.  This means 
you cannot exclude guava from the metron-statistics dependency and the version 
mismatch tends to show up at runtime.  I believe this problem is outside the 
scope of this PR and should be fixed separately.  There are other modules that 
have the same problem so those need to be fixed as well.

Speaking of guava, you might want to consider not using it in 
KafkaServiceImplTest.  That library is deprecated aggressively and has caused 
us a lot of problems because we depend on so many different projects that all 
use different guava versions.  Not a big deal if we do use it (we're using it 
directly in other Metron modules) but we might want to call that dependency out 
and use the Metron global guava version.  It looks like you're just using it 
for syntactical convenience and isn't critical so my preference would be to 
take it out.  I could be convinced otherwise if you think we'll need it down 
the road.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr as we discussed, I created a PR against yours with some unit 
tests for the services. Let me know if you have any questions.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Conflict is resolved


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Those 2 are done.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr this is looking good. I like how you broke out some of the 
services and started a constants class. There were two very minor things that I 
noticed.

1. `@Override` should be used wherever possible on all service class method 
that implement an interface.
2. I don't believe `@Service` is needed on the interfaces.

I continued to test some more endpoints and things are continuing to look 
good. I still want to test out some more endpoints.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@jjmeyer0, just pushed out the commit to separate Services into interfaces 
and implementations.  As I created the interfaces there were a few spots that 
needed some changes.  

First there were several methods in the GrokService that didn't belong 
there.  These include methods that support writing grok statements to/from HDFS 
and updating a sensor parser config with these statements.  I believe these 
methods should be in SensorParserConfigService since they are needed to support 
writing and reading sensor parser configs.  It should be noted that these 
methods can all be removed if we ever move to keeping grok statements in 
zookeeper instead of HDFS (see PR #308).

The second is something you alluded to earlier about converting the 
StormService to a facade pattern.  I think you are correct there, this service 
contains 2 different types of Storm interactions:

- function calls to the Storm REST API
- functions that are not exposed through the Storm REST API, only through 
the Storm CLI

I broke this service down into 2 smaller services.  Is that along the lines 
of what you were thinking?

I also moved all the constants in various classes to a single constant 
class.  We don't have that many right now so it's manageable to keep them in 
one place.  

Was also thinking we should rename TransformationService to StellarService? 
 I can see that evolving to be more generic than just the Stellar hook in the 
transformation phase of the parser topology.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Just pushed out a commit to address all the issues noted above with the 
exception of separating the service classes into interfaces and 
implementations.  

For that, @jjmeyer0 is there a pattern you prefer or have used before that 
worked well for you?  To extend on the example I gave above (SomeService and 
SomeServiceImpl), we would also need to add something like a ServiceConfig 
class that instantiates all of the Impl classes to be injected wherever the 
various services are needed.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@jjmeyer0 #2 make sense.  That would be great if you can open another Jira 
to discuss.

Can you clarify what you mean on number 1?  Are you talking about 
separating the services into something like SomeService and SomeServiceImpl?


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
I've tested a few controllers (global config, grok controller, and kafka 
controller). Everything seems to be working pretty well. I will continue to 
test more, but since this is a large PR I wanted to do smaller chunks at a time.

I think we should start thinking about is how we are going to architect the 
service layer. I'd like to discuss two things that I think will help us improve 
the maintainability of this.

*Each service should implement an interface.**

**We should discuss how to better break up services.** 
I think services should be conceptually simple. Mostly interact with a 
specific system. If we start getting into more complicated situations where 
multiple services are needed these should be in a separate area. I consider 
situations like these to be more of a service facade. A current example of this 
is with the `StormService`. I think the interactions with the storm API should 
be in its own service. Then a facade would be used with the storm service and 
other necessary services to give a simpler view into storm based on our 
context. Does this make sense?

Personally I think number 1 should be done apart of this PR and number 2 
should probably turn into a wider discussion. What do you all think? If others 
agree, I'd be glad to write up a Jira to discuss number 2.



---
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.
---

Re: [GitHub] incubator-metron issue #316: METRON-503: Metron REST API

[Otto Fowler]

Ryan,

I owe you an apology, that discussion is more towards the web app pr and
not the rest api which would still be consumed by an ambari view or a
custom app.

I confused the threads.



On January 19, 2017 at 09:29:29, merrimanr (g...@git.apache.org) wrote:

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316

I would very much prefer to not wait. There are other endpoints outside of
configuration management that we need, a REST API isn't limited to just
that. If we feel we should remove the auditing functions I am happy to do
that. With the auditing functions removed, there will not be major changes
needed once configuration management is moved to Ambari.

@ottobackwards, I don't understand what you mean by this:

"Also, the point that this may be better managed from within ambari as a
view may need consideration."

Can you expand on that?


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
The primary driver as I see it for a REST API is to facilitate building 
user interfaces for various interactions with Metron.  At some point (I would 
argue now) we will want/need to evolve from just exposing functionality through 
a collection of command line utilities spread out across various hosts.  That 
is not user friendly at all and anyone outside of our community of 
committers/contributors will have a steep learning curve.

Most modern web development frameworks that I know of have all converged to 
REST and JSON standards for calling APIs.  So to build a UI or encourage others 
in the community to build UIs, we need to expose functions in Metron through 
REST/JSON.  Some functions are already exposed through a REST interface (the 
Storm REST API for example).  But there are several gaps.  A few that come to 
mind:

- you cannot start/stop topologies through the Storm REST API
- we are not managing sensor specific configs (parser, enrichment, 
indexing) with Ambari yet so the only way to do that is with the 
zk_load_configs.sh script
- the only way to execute Stellar statements outside of topologies is with 
the Stellar REPL

There are many more and there will no doubt be more that come up as the 
platform evolves.  We need a middle-tier to fill in these gaps and grow our 
library of functions that users and developers can leverage.  

We've all agreed that managing sensor specific configs in Ambari has a lot 
of merit.  When the time comes and that actually gets implemented, it's fairly 
trivial to either rip those endpoints out or change the underlying 
implementation.  The REST API as it stands in this PR, makes the same exact 
calls as the zk_load_configs.sh script so the changes are likely minimal (they 
will have to be made to zk_load_configs.sh anyways).  Removing the auditing 
endpoint in anticipation of moving configs to Ambari is perfectly reasonable.  
I am happy to update this PR with that change.

There are probably further discussions we can have with respect to our REST 
strategy but right now we are missing a critical piece in our architecture.  
Even after we get the configs moved to Ambari we will still need this so I 
don't see any reason to hold it up.  If any of this is unclear I am happy to 
explain further and provide additional documentation or diagrams. 


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
I would very much prefer to not wait.  There are other endpoints outside of 
configuration management that we need, a REST API isn't limited to just that.  
If we feel we should remove the auditing functions I am happy to do that.  With 
the auditing functions removed, there will not be major changes needed once 
configuration management is moved to Ambari.

@ottobackwards, I don't understand what you mean by this:

"Also, the point that this may be better managed from within ambari as a
view may need consideration."

Can you expand on that?


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@ottobackwards that's a good point. To me it still may makes sense to have 
this API (which the Ambari view would use). I think it would be useful for 
clusters not using Ambari. But I suspect this is going to be a much longer 
discussion.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Also, the point that this may be better managed from within ambari as a
view may need consideration.



On January 19, 2017 at 08:58:51, JJ Meyer (notificati...@github.com) wrote:

@merrimanr  @cestella
 does it make sense to hold off on this for a
little while? It looks like all configuration may be managed by Ambari
going forward. I am getting this from the dev thread: Ambari Metron
Configuration Management consequences and call to action.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub

,
or mute the thread


.



---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr @cestella does it make sense to hold off on this for a little 
while? It looks like all configuration may be managed by Ambari going forward. 
I am getting this from the dev thread: `Ambari Metron Configuration Management 
consequences and call to action`. 


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
That use case makes sense.  I will leave it in.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr, the reasoning I used to include the http status code in the 
error message format is if there is a need to send these messages to a 
downstream system. For example, let's say a developer is integrating with the 
Metron API. For some reason they want to drop all error responses on a queue 
for processing by another system. If the status code didn't exist in the error 
format some context would be lost. But to your point maybe it doesn't buy us 
much. I don't have a strong preference either way on this one. However, maybe 
at some point it may be worth having a custom attribute called `code` that 
would allow a user to look up the errors in documentation. It could potentially 
show things like common causes and workarounds. That sounds like a separate PR 
with a lot of discussion around it though.


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Just pushed out a commit to address Response Behavior.  I defined an error 
response type using the structure you proposed and added a RestExceptionHandler 
as described in the link to the Spring Boot docs above.  I think it's a good 
solution.  One question though, do we need the "responseCode" field?  Won't it 
always match the Http status code?

Also added the metron-rest-client module.




---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@merrimanr yes, I was thinking of something along these lines. At a high 
level I was thinking that any exception that bubbled up through the controller 
should be mapped to an error response. I believe Spring uses 
`ResponseEntityExceptionHandler` to help with this 
(http://docs.spring.io/spring-boot/docs/1.4.1.RELEASE/reference/htmlsingle/#boot-features-error-handling).
 The benefits I see from doing this are keeping the error handling consistent, 
separating concerns, and giving better feedback to the end user. It does 
require more thought on what exceptions are thrown, and if the exception 
handler has the ability to map them to a response . Does this make sense?



---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
@jjmeyer0 thank you for the feedback, I think they are good suggestions.  
Just pushed out commits to address "Project Structure", "API Documentation", 
and "API Structure".  Let me know what you think.

For the Response Behavior, I agree it's somewhat subjective.  I'm happy to 
discuss it now and add it into this PR.   Let me see if I understand you 
correctly, trying to map your suggestions to an Http response.  In the example 
you give, are you proposing (in the case of a missing config or something) that 
the response code be 404 and the response body be an object with "message" and 
"fullMessage" fields?  


---
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] incubator-metron issue #316: METRON-503: Metron REST API

[jjmeyer0]

Github user jjmeyer0 commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  

**Project Structure:**

Personally I think the module, `metron-interface`, should be split up into 
more modules. I think there should be `metron-rest` and `metron-rest-client`. 
The idea would be to move the models into the client module. Specifically the 
models returned by the endpoints and are used when making requests. I 
personally like to to do this because it allows a user to use the client 
without having to bring a ton of dependencies onto their classpath. This is 
assuming there will eventually be a client module. Does this make sense? Think 
it is worth doing now?

**Response Behavior:**

I think we should also decide how Metron's APIs handle errors. In the past 
I have defined an error response type that was used by all endpoints. It could 
potentially look something like:

{
  "responseCode" : 404, 
  "message" : "Could not find parser config.", 
  'fullMessage" : "Could not find parser config with the name: [some name]"
}

Also, I personally do not like to throw exceptions of type `Exception`. I 
think we could do a couple things. We could create a MetronRestException that 
gets mapped to an error response. We could also have the service layer use an 
`Either` type which would either return the response entity or a set of error 
responses. Do you all think this is worth talking about now? I think this is 
always one of those things that's tough to decide, but should be standard 
across the API.

**API Documentation:**

I think it is worth documenting all the different response types for each 
endpoint. For example, the endpoint `/api/v1/sensorParserConfigHistory` only 
describes the response for a 200 code, but there is also a 404. This can be 
done by using Swagger's `ApiResponses` annotation.

**API Structure:**

As for API structure there were a few that I thought could potentially be 
changed. Below are a few examples. To me it may make sense at some point to 
have a sensor endpoint. IMO breaking it up as I did below groups them a bit 
more nicely (isn't an exhaustive list). What do you think? 

/api/v1/globalConfig -> /api/v1/global/config
/api/v1/sensorEnrichmentConfig -> /api/v1/sensor/enrichment/config
/api/v1/sensorParserConfig -> /api/v1/sensor/parser/config
/api/v1/sensorParserConfigHistory -> /api/v1/sensor/parser/config/history



---
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] incubator-metron issue #316: METRON-503: Metron REST API

[merrimanr]

Github user merrimanr commented on the issue:

https://github.com/apache/incubator-metron/pull/316
  
Just updated this PR to include several changes/improvements:

- Licensing documentation
- Expanded to include more services: Kafka, SensorEnrichmentConfig, 
GlobalConfig, Storm, Grok, Stellar Transformations
- Basic security
- Swagger for testing and interacting with the API
- Docker Spring profile for running against a Metron Docker environment

Documentation for installing the REST service is included in the README.  
To test in an IDE against a Metron Docker environment (assuming that is 
running):

- change the MySQL and Hibernate dependencies from provided to compile
- Use org.apache.metron.rest.MetronRestApplication as the main class
- pass in "--spring.profiles.active=docker" as an argument or set the 
Spring profile to "docker" in your IDE

During a review of the licenses we learned that the Hibernate license is 
not Apache friendly.  I am planning to substitute EclipseLink as the JPA 
implementation in the future.  This is why the Hibernate installation is a 
separate install step.



---
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.
---