[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-26 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
+1 

Overall looks good and works as expected. I've built with contrib check and 
tested using a vanilla instance. I will squash and merge to master. Thanks 
@MikeThomsen.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-26 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@JPercivall I just got done testing it w/ X-Pack for SSL and u/p auth based 
on Elastic docs. Everything was in working order there. [Steps if you want to 
validate](https://www.elastic.co/guide/en/elasticsearch/reference/6.2/configuring-tls.html)

I also added the documentation there. Sorry about that.

AFAIK, that warning is going to be more of a polite warning than a real 
"here be dragons" sort of thing because IIRC ES limits you to 10k results 
unless you explicitly up the number in the index configuration. Probably a good 
reminder, but I doubt anyone is going to blow up their NiFi installation 
without having to take deliberate steps to facilitate that.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-25 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Looks good, was able to do a couple simple queries and aggregations. I 
wasn't able to test HTTPS though as I don't have access to the X-Pack. Were you 
able to do so @MikeThomsen?

The only lingering issue, that I've mentioned multiple times now, is to add 
a note about the entire body of the response being loaded into memory.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-24 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@JPercivall I fixed the dependency issue and tried a simple flow. 
Everything seemed to work.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-23 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Hey @MikeThomsen, just reviewed the changes. They look good and I think 
we're almost there. One thing that is missing is the comment regarding pulling 
the results into memory.

Also, I tried to build NiFi and test the processor an instance but hit the 
following error when I attempted to start up NiFi (./bin/nifi.sh start). I 
tried rebasing it to the latest master but to no avail. 

> 2018-03-24 02:06:53,171 ERROR [main] org.apache.nifi.NiFi Failure to 
launch NiFi due to java.util.ServiceConfigurationError: 
org.apache.nifi.processor.Processor: Provider 
org.apache.nifi.processors.elasticsearch.JsonQueryElasticsearch could not be 
instantiated
java.util.ServiceConfigurationError: org.apache.nifi.processor.Processor: 
Provider org.apache.nifi.processors.elasticsearch.JsonQueryElasticsearch could 
not be instantiated
at java.util.ServiceLoader.fail(ServiceLoader.java:232)
at java.util.ServiceLoader.access$100(ServiceLoader.java:185)
at 
java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:384)
at java.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:404)
at java.util.ServiceLoader$1.next(ServiceLoader.java:480)
at 
org.apache.nifi.nar.ExtensionManager.loadExtensions(ExtensionManager.java:148)
at 
org.apache.nifi.nar.ExtensionManager.discoverExtensions(ExtensionManager.java:123)
at org.apache.nifi.web.server.JettyServer.start(JettyServer.java:771)
at org.apache.nifi.NiFi.(NiFi.java:157)
at org.apache.nifi.NiFi.(NiFi.java:71)
at org.apache.nifi.NiFi.main(NiFi.java:292)
Caused by: java.lang.NoClassDefFoundError: 
com/fasterxml/jackson/databind/ObjectMapper
at 
org.apache.nifi.processors.elasticsearch.JsonQueryElasticsearch.(JsonQueryElasticsearch.java:206)
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
at java.lang.Class.newInstance(Class.java:442)
at 
java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:380)
... 8 common frames omitted
Caused by: java.lang.ClassNotFoundException: 
com.fasterxml.jackson.databind.ObjectMapper
at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
... 15 common frames omitted


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-22 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@JPercivall FYI, part of the reason I want to get this done is I'm planning 
on doing a whole new set of ES processors that are based on bring the total 
CRUD functionality over to the official Elastic-provided APIs. The current set 
of processors use the transport API (deprecated) and make manual REST calls 
that AFAIK don't do master detection and stuff like that which comes baked-in 
with the Elastic APIs.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-22 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Thanks.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-22 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Hey @MikeThomsen, I'm planning on reviewing this tomorrow evening


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-22 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@JPercivall Any chance we can close this out?


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-19 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@JPercivall changes checked in.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-18 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
> I hope my review doesn't come off in the wrong way, there is a lot of 
great work here and I just want to make sure the usability is top notch.

Not at all. It's all fair and good feedback. I'll take a crack at these 
tomorrow.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-18 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Hey @MikeThomsen, just finished up another round of reviewing. Again, 
mostly focused on the error handling portion. 

I hope my review doesn't come off in the wrong way, there is a lot of great 
work here and I just want to make sure the usability is top notch. Error 
handling is a huge focus in NiFi because we're writing up a toolbox of 
processors for potentially non-technical users to work with however they want. 
Those users need to be able to see what they messed up and what went wrong for 
efficient and effective rectification. 

Thanks for the hard work, and the willingness to iterate and learn!


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-18 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Taking a look now. WRT:

> Ok, I can do that from now on. Shouldn't they be squashed before a merge 
into master?

Yup, they will be squashed prior to merge to master but the reviewer can do 
that at the same time they add the comment to the commit to close the PR.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-17 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Ok. It should all be there now.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-17 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@JPercivall Thanks for the feedback. I'll get to working on these. WRT:

> Lastly, it's preferred if you don't squash your commits every time. If 
you don't, that allows the reviewer to more easily see exactly what changed 
since they last reviewed it. Also allows reviewers to see how the PR evolved 
over time in response to different comments.

Ok, I can do that from now on. Shouldn't they be squashed before a merge 
into master?


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-17 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Thanks @MikeThomsen, looks like that fixed the NPE. Still needs the License 
and Notice in the client service nar.

Also, it's a bit of an anti-pattern to have the logging and error handling 
in the Controller Service for methods called by the processor. This is because 
any time there is a problem with the search, the bulletin is logged at the 
Controller Service. This means that not only will a user not know which 
processor is the cause of the error (since it could be shared) but also when 
looking at the map the processor appears to be working without issue. Lastly, 
since the error isn't bubbled back to the processor, it never yields so if 
there is something wrong with the query it will continually error and log 
without pause. 

Related, there's also some weirdness with error handling when there is an 
incoming connection. Currently, input is only sent to Failure instead of 
Original when an error is caught at the top level. This means that if a query 
is messed up and is caught as a JsonProcessingException in the Controller 
service, the input will be routed to original and no other outputs. As a user, 
I would expect that flowfile to be sent to Failure.

So in summary, the CS should let the calling processor decide how to handle 
any exceptions encountered during processing. This allows the processor to more 
granularlly decide what to do under different situations. For example, was it a 
JSON parsing exception and it's acting as a source processor? well we better 
yield the whole processor since it's probably just going to continually fail. 
Is there an incoming connection but some network error happened when searching? 
again, later FlowFiles will probably fail too, so we should yield. If you have 
any questions on when to yield the processor vs penalizing the FlowFile, feel 
free to ask.

Lastly, it's preferred if you don't squash your commits every time. If you 
don't, that allows the reviewer to more easily see exactly what changed since 
they last reviewed it. Also allows reviewers to see how the PR evolved over 
time in response to different comments.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-17 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@JPercivall Done.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-15 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Apologises, won't be able to finish the review this evening. I reviewed the 
restapi nar License and Notice files they look good to go. Just need to do a 
bit of functional testing and verify @mattyb149 comments were addressed and we 
should be ready to merge (pending the L/N for the service nar).


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-15 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
The nifi-elasticsearch-client-service-nar is missing a license and notice.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-15 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Thanks @MikeThomsen I should be able to take another look later tonight


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-15 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@JPercivall It's ready for review.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-13 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
2/3 builds pass now.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-13 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@JPercivall @mattyb149 Once I changed the code to use `transfer(FlowFile, 
Relationship)` and not `transfer(List, Relationship)` the error stopped 
happening.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-13 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@JPercivall @mattyb149 That exception only happens for me when I stop the 
processor. It happens here:

``
if (hitsFlowFiles.size() > 0) {
session.transfer(hitsFlowFiles, REL_HITS);
for (FlowFile ff : hitsFlowFiles) {
session.getProvenanceReporter().send(ff, 
clientService.getTransitUrl(index, type));
}
}
``

On the provenance reporting line. Any ideas?


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-12 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
I'm seeing NPE and FlowFile handling exceptions. I didn't do anything 
special, just using the example query. I'll attach a template. Errors from the 
logs below.


> 2018-03-12 23:58:11,386 ERROR [Timer-Driven Process Thread-10] 
o.a.n.p.e.JsonQueryElasticsearch 
JsonQueryElasticsearch[id=1d5d83d1-0162-1000-cdf2-92fa9e61ef42] Error 
processing flowfile.: java.lang.NullPointerException
java.lang.NullPointerException: null
at 
org.apache.nifi.processors.elasticsearch.JsonQueryElasticsearch.onTrigger(JsonQueryElasticsearch.java:248)
at 
org.apache.nifi.processor.AbstractProcessor.onTrigger(AbstractProcessor.java:27)
at 
org.apache.nifi.controller.StandardProcessorNode.onTrigger(StandardProcessorNode.java:1123)
at 
org.apache.nifi.controller.tasks.ContinuallyRunProcessorTask.call(ContinuallyRunProcessorTask.java:147)
at 
org.apache.nifi.controller.tasks.ContinuallyRunProcessorTask.call(ContinuallyRunProcessorTask.java:47)
at 
org.apache.nifi.controller.scheduling.TimerDrivenSchedulingAgent$1.run(TimerDrivenSchedulingAgent.java:128)
at 
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
at 
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
at 
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
2018-03-12 23:58:11,389 ERROR [Timer-Driven Process Thread-10] 
o.a.n.p.e.JsonQueryElasticsearch 
JsonQueryElasticsearch[id=1d5d83d1-0162-1000-cdf2-92fa9e61ef42] 
JsonQueryElasticsearch[id=1d5d83d1-0162-1000-cdf2-92fa9e61ef42] failed to 
process due to org.apache.nifi.processor.exception.FlowFileHandlingException: 
StandardFlowFileRecord[uuid=77ad5fff-d4b6-4e11-8d8c-b07ac1d0a6cb,claim=StandardContentClaim
 [resourceClaim=StandardResourceClaim[id=1520913052434-1, container=default, 
section=1], offset=30, length=228],offset=226,name=46172028496405,size=2] 
transfer relationship not specified; rolling back session: {}
org.apache.nifi.processor.exception.FlowFileHandlingException: 
StandardFlowFileRecord[uuid=77ad5fff-d4b6-4e11-8d8c-b07ac1d0a6cb,claim=StandardContentClaim
 [resourceClaim=StandardResourceClaim[id=1520913052434-1, container=default, 
section=1], offset=30, length=228],offset=226,name=46172028496405,size=2] 
transfer relationship not specified
at 
org.apache.nifi.controller.repository.StandardProcessSession.checkpoint(StandardProcessSession.java:251)
at 
org.apache.nifi.controller.repository.StandardProcessSession.commit(StandardProcessSession.java:321)
at 
org.apache.nifi.processor.AbstractProcessor.onTrigger(AbstractProcessor.java:28)
at 
org.apache.nifi.controller.StandardProcessorNode.onTrigger(StandardProcessorNode.java:1123)
at 
org.apache.nifi.controller.tasks.ContinuallyRunProcessorTask.call(ContinuallyRunProcessorTask.java:147)
at 
org.apache.nifi.controller.tasks.ContinuallyRunProcessorTask.call(ContinuallyRunProcessorTask.java:47)
at 
org.apache.nifi.controller.scheduling.TimerDrivenSchedulingAgent$1.run(TimerDrivenSchedulingAgent.java:128)
at 
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
at 
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
at 
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-12 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Template for FlowFile handling exception.


[FlowFile_Handling_Exception.txt](https://github.com/apache/nifi/files/1805420/FlowFile_Handling_Exception.txt)



---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-12 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Looks like the master build is failing with some check style issues, hence 
why your builds failed. I'll work on fixing them.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-12 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Reviewing


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-12 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Rebased and building. @mattyb149 I changed the functionality to match the 
GetMongo functionality you reviewed recently. Had to spend a while getting the 
build to work again.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-12 Thread mattyb149
Github user mattyb149 commented on the issue:

https://github.com/apache/nifi/pull/2113
  
I'm not sure I'll have time to close the loop before 1.6.0, so if you'd 
like to finish the review/merge after Mike's rebase that would be very cool, 
thanks!


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-03-12 Thread JPercivall
Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Hey @mattyb149, what's your status for this review? From a cursory look, it 
appears like it just needs an updated pom from @MikeThomsen and the final 
approval. With talks of 1.6.0 happening it would be nice to get this in so 
those using ES 6 aren't limited to the HTTP processor. 

If help is needed to finalize things just let me know where I can help.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-02-05 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@mattyb149 I think all of the change requests are done.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-01-22 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@mattyb149 I moved it to a new NAR and rebased against the latest master 
last night. So once this is committed, I'll start working on the new processors 
and service methods.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-01-16 Thread mattyb149
Github user mattyb149 commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Makes sense to me, we had to do the same with Elasticsearch 2.x and 5.x 
because of the difference in transport client versions. As folks move 
exclusively to ES5+, it would be nice to deprecate the other NARs and go 
forward with versions of all the processors (Get,Put,Search, etc.) using the 
new Java client.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2018-01-16 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@mattyb149 I've been thinking about what to do with this PR, and I think it 
needs to be put into a new NAR. I'm thinking "nifi-elasticsearch-restapi.nar" 
or something like that. What I'd ideally like to do is position this to be a 
new package that does full CRUD using only the official REST API from Elastic 
Co.

It might also help with the issue that @joewitt raised about the sizing 
because the current mix of ElasticSearch processors use different toolkits 
which needlessly raises the size of the NARs.

Thoughts?


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-12-22 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@mattyb149  I think I got your changes done now. I renamed a few classes to 
remove obvious references to ElasticSearch 5.X because the goal is to make it 
an obvious fit with 6.X too now that that is out.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-12-01 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@mattyb149 I think everything should be up to date now.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-11-12 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@mattyb149 I think everything except the proxy issue is now in there.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-11-09 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Thanks @mattyb149 I'll try to find some time to address these today or 
tomorrow. I've been knee deep in some Mongo-related work for a client. I have 
two pull requests for Mongo and record api-related fixes/updates if you're up 
for reviewing two small patches (one is 3 lines of code :) )


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-11-03 Thread mattyb149
Github user mattyb149 commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Reviewing...


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-10-25 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@joewitt alright, I'll see what trouble I can get into.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-10-24 Thread joewitt
Github user joewitt commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@MikeThomsen It is awesome that you're contributing code/PRs.  As you can 
see though we do have quite a large backlog of PRs that need 
review/feedback/cycles.  Some of them, like integration with Mongo involve more 
effort than others to help test and review.  Please do consider helping by 
participating in/conducting reviews of others contributions as well.  That 
might help things like this be evaluated faster as there would be less overall 
backlog.  Thanks


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-10-24 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@mattyb149 Have you had a chance to take a look at it?


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-10-17 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@mattyb149 Ok. Changes are made. I refactored the commit to be based on a 
controller service. For now, that service only handles a single function: basic 
search. However, there is now a (hopefully) extensible base via a controller 
service to move forward. The controller service has integration tests only; the 
processor has junit tests with a mock service.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-10-13 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
@mattyb149 I'm going to leave this open, but I decided to refactor the heck 
out of it around a client service for ElasticSearch. The service only has one 
method for now, but I think it's the way to go so that in the future as 
services become injectable in scripts and such, it'll be more flexible.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-10-09 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
I set up a profile called integration-tests and renamed the file to 
JsonQueryElasticsearch_IT. It runs really well from the project folder, but for 
some reason running `mvn clean install -Pintegration-tests -pl 
:nifi-elasticsearch-5-processors` from the root folder causes surefire to be 
run twice (the first time passing, the second time failing because it stops 
ElasticSearch in between the two runs). When run from 
nifi-elasticsearch-5-processors it executes as expected.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-10-09 Thread mattyb149
Github user mattyb149 commented on the issue:

https://github.com/apache/nifi/pull/2113
  
If you're looking to run integration tests against a ES instance (from the 
Maven plugin), perhaps you can split out the integration tests into a new file 
called ITJsonQueryElasticsearch5, rather than having the @Ignore or running 
integration tests during the unit test phase. I realize the others aren't done 
this way, but if that Maven plugin works well, I think we should consider a 
separate Jira/PR to refactor the tests into proper integration vs unit tests, 
so they can be run during their corresponding lifecycle (integration tests 
aren't run during a normal test phase, e.g.)


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-10-08 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
I discovered a slick plugin for Maven that lets you cleanly run 
ElasticSearch in the background during integration tests. I added that to the 
test setup under a profile. So I have some cleanup to do on this branch to 
bring that in. I think it'll be worth it.


---


[GitHub] nifi issue #2113: NIFI-4325 Added new processor that uses the JSON DSL.

2017-10-07 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2113
  
Thanks. I'll get to these some time today or tomorrow.

FYI Elastic has some official statements on the future direction of the 
Java client here: 
https://www.elastic.co/blog/state-of-the-official-elasticsearch-java-clients

The other processors will need reviewing to make sure they look good.


---