[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user mans2singh commented on the issue: https://github.com/apache/nifi/pull/2562 @MikeThomsen @joewitt - Thanks for your help in making this contribution possible. @timhallinflux - Please let me know if you have any additional enhancements possible. I will contact you via email. Thanks again everyone. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user timhallinflux commented on the issue: https://github.com/apache/nifi/pull/2562 Yep. Working on a blog post to highlight both parts...writer and reader. U is not really possible. For D, both DELETE and/or DROP statements are allowed through InfluxQL. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2562 @timhallinflux Based on my extremely limited knowledge of what InfluxDB can do, I don't know if even the U or D from CRUD are really applicable to InfluxDB for example. What I'd like to see is some input that would help us mold this bundle to maxing out the reasonable use cases where NiFi and InfluxDB can sensibly work together if that makes sense. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2562 @timhallinflux if you don't mind, please take some time to think about additional NiFi-related use cases and either post them to the dev mailing list or add them as Jira tickets. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user timhallinflux commented on the issue: https://github.com/apache/nifi/pull/2562 This is great! Thank you Mike and Mans. Mans if you reach out to me at tim at influxdata dot com we would be happy to send you some stickers and a hoodie in appreciation for your efforts here. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2562 Approved and merged. Thanks @mans2singh ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user mans2singh commented on the issue: https://github.com/apache/nifi/pull/2562 @MikeThomsen - I've updated the code (added expression language scope and updated tests) based your review comments. The integration tests are passing. Please let me know if there is any other comment. Thanks Mans ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2562
@mans2singh I went to merge your changes this morning and found that you
have some integration tests that are broken now with the changes made to EL
support in the `PropertyDescriptor.Builder` class. It seems your breakage is
limited mainly here:
```
protected String makeProvenanceUrl(final ProcessContext context, String
database, FlowFile flowFile) {
return new StringBuilder("influxdb://")
.append(context.getProperty(INFLUX_DB_URL).evaluateAttributeExpressions(flowFile).getValue()).append("/")
.append(database).toString();
}
```
You marked that property as `VARIABLE_REGISTRY` and it is failing because
you're applying it flowfiles. The existing calls to
`expressionLanguageSupported(boolean)` in the abstract class also need to be
fixed.
So just do a rebase against the latest master and take care of those. If
there are no regressions after your commit (ie your flows and tests run) IMO
we're ready to merge.
---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user mans2singh commented on the issue: https://github.com/apache/nifi/pull/2562 Thanks @MikeThomsen for your reveiw/advice. Also thanks @timhallinflux for the pointers on influxdb sandbox. Please let me know if there is other feedback. Mans ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2562 Forgot to add that a link to a comprehensive sandbox is appreciated @timhallinflux ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2562 @timhallinflux @mans2singh already provided a decent enough Docker Compose test for it. So unless @joewitt or someone else sees something that I missed that's a show stopper, I think we're good to go on merging this to master when 1.6 is released into the wild. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user timhallinflux commented on the issue: https://github.com/apache/nifi/pull/2562 @joewitt and @MikeThomsen -- you can also just quickly spin up the TICK stack via the sandbox. Have a look here... https://github.com/influxdata/sandbox If @joewitt is no longer technical enough to run this, I'll be more than happy to do a screen share and help him out! :-) Thank you @mans2singh for contributing this! ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user mans2singh commented on the issue: https://github.com/apache/nifi/pull/2562 @MikeThomsen - I have updated the test cases regarding usage of assertions. Let me know if there is anything else outstanding. Thanks ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user mans2singh commented on the issue: https://github.com/apache/nifi/pull/2562 @MikeThomsen - I've updated the code based on your comments. Let me know if you have any more recommendations. Thanks ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2562 Verified that both flows work against the Docker Compose configuration. Looking very good so far. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user mans2singh commented on the issue: https://github.com/apache/nifi/pull/2562 @MikeThomsen - I've updated the code based on your comments (added check for query result null, changed scheduled query property name). One note, if we use the influxdb-compose.xml file for integration testing, we will need to change the name of the db (from test to something else) which the curl loader uses, since integration test use their own data. Please let me know your thoughts/recommendations. Thanks. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user mans2singh commented on the issue: https://github.com/apache/nifi/pull/2562 @joewitt @MikeThomsen - I've updated the code based on your comments. I've also added a docker gist [influxdb-compose.xml](https://gist.github.com/mans2singh/5ee90620314d4bcf26d6c65de2540c77#file-influxdb-compose-xml) based on docker compose xml mentioned by @MikeThomsen . This docker compose starts a local influxdb and runs curl commands to create a `test` database and populate it with a metric every 5 seconds. To launch it we need to run `docker-compose -f up` on the local machine and then we can run the integration tests for the query processor or run Nifi locally with processor configured for scheduled query. I am also including two templates ([flow file driven](https://gist.github.com/mans2singh/5ee90620314d4bcf26d6c65de2540c77#file-nifi-influxdb-flow-file-driven-template) and [timer driven](https://gist.github.com/mans2singh/5ee90620314d4bcf26d6c65de2540c77#file-nifi-influxdb-scheduled-query-template)) to assist in testing the InfluxDB query processor. Please let me know if your comments/recommendations. Thanks again. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user mans2singh commented on the issue: https://github.com/apache/nifi/pull/2562 Hi @MikeThomsen - Thanks for your review and comments. I will work on your and @joewitt 's recommendation this weekend. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2562 @mans2singh Have you had a chance to work on any of these? ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2562 @joewitt @mans2singh [Here's a starting point](https://github.com/apache/nifi/pull/2101#issuecomment-361083921) on the Docker/Docker Compose ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user joewitt commented on the issue: https://github.com/apache/nifi/pull/2562 @mans2singh thanks for contributing. What we need most right now is help reviewing outstanding PRs. This involves reviewing code and validating as possible whether things are actually functioning. For things like this it can be tougher because it means someone needs an environment with influx running/etc.. and familiarity on how to do so. What can you do? You can provide detailed instructions a reviewer could follow to verify functionality. Perhaps a pointer to a docker container which eases the process. And help with reviews on other items, for example. Not saying someone wont look at this until then - they certainly might. Just giving you tips to help ease the process. ---
[GitHub] nifi issue #2562: NIFI-4927 - InfluxDB Query Processor
Github user mans2singh commented on the issue: https://github.com/apache/nifi/pull/2562 Hi Nifi Team: Please let me know your suggestions/recommendations on this InfluxDB Query Processsor. Thanks ---
