Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/3116
Thanks, @ijokarumawak and @adamlamar! The combined change looks good to
me.
I ran it through multiple test loops putting and listing about 10,000
objects in S3. No objects were missed
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2361
Thanks, @adamlamar!
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/3116
Reviewing...
---
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2859#discussion_r204968906
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/sqs/PutSQS.java
---
@@ -115,7 +125,19 @@ public
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2859#discussion_r204246421
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/sqs/PutSQS.java
---
@@ -115,7 +125,19 @@ public
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2859#discussion_r204246953
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/sqs/PutSQS.java
---
@@ -57,6 +59,14
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2859
Reviewing...
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2825
Thanks @lfrancke
---
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2751#discussion_r194140856
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/ListS3.java
---
@@ -307,6 +328,20 @@ private
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2704#discussion_r189164826
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/test/java/org/apache/nifi/processors/aws/s3/TestListS3.java
---
@@ -311,5 +312,10
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2704#discussion_r189164994
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/AbstractAWSProcessor.java
---
@@ -92,6
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2704#discussion_r189164964
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/AbstractAWSProcessor.java
---
@@ -92,6
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1888
Thanks, @pvillard31
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2665
Thanks, @zenfenan, this looks good.
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2665
Reviewing...
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2664
Thanks, @zenfenan, this looks good.
- Full build with contrib-check passes
- nifi-aws-processors integration tests pass
- Manual testing with an S3 flow worked fine
And great news
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2664
Reviewing...
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2556
LGTM:
* Full build passes
* Good use of SystemResourceConsideration for memory consideration
* Good test coverage
* PutEmail worked correctly in my testing with various content option
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2556
Reviewing
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2506
@pvillard31 - Would you please close the PR? I merged the change, but
fat-fingered the PR number in the commit message.
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2506
Thanks for the update @pvillard31. Thanks for reviewing, @zenfenan.
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2505
The extra close is a result of my carelessness, sorry. That should have
been for PR #2506.
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2491
Thanks, @pvillard31
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2492
Reviewing...
---
GitHub user jvwing opened a pull request:
https://github.com/apache/nifi/pull/2491
NIFI-4876 Adding Min Object Age to ListS3
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2409
@SunSatION, this is nice work. Thanks for the new Kinesis expression
language features. And double thanks for fixing the if statements and the
integration tests, I think your Jackson test dependency
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2409
@SunSatION , thanks for submitting this PR, it looks like good stuff.
May I ask how you tested the changes? For PutKinesisFirehose, I was able
to run the integration test suite successfully
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2409#discussion_r162796692
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/kinesis/firehose/PutKinesisFirehose.java
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2409#discussion_r162796710
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/kinesis/stream/PutKinesisStream.java
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2409
Reviewing...
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2017
Thanks, @trixpan.
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2344
Thanks, @mgaido91! The code change looks good and works good. And thanks
for tuning up the tests appropriately.
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2344
Reviewing...
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2321
@edwardzjl - Is there a JIRA ticket for this issue? If so, would you
please update the PR title with it. If not, would you please create one (at
https://issues.apache.org/jira/browse/NIFI/)? JIRA
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2291
@baank, did you mean to close this PR? The latest changes appear to be
well worth reviewing, if that is OK with you?
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2300
@aburkard Thanks again for putting this together.
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2291
@baank - closed?
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2291
@baank - thanks for the update, I think we're almost done. Two things:
1.) I still see a checkstyle warning for nifi-aws-processors:
> [INFO] --- maven-checkstyle-plugin:2.15:ch
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2291#discussion_r153396888
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/PutS3Object.java
---
@@ -154,6 +155,14
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2291#discussion_r153396684
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2291#discussion_r153398470
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/encryption/EncryptedS3PutEnrichmentService.java
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2291
@baank, thanks for the latest update. Good news, we're getting down to the
nit-picks:
1. I had a checkstyle error running the full build with contrib check on
nifi-aws-service-api
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2248
@baank, thanks for putting in all the work on this PR so far, it is looking
pretty good. The full build with contrib-check passed. I have been able to
run through the encryption functionality
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2248
Reviewing
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2140
@christophercurrie, thanks again for the work on this infrastructure
project.
I merged these commits to master, except for the last commit "Limit NOTICE
to AWS SDK" for nifi-aws-s
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2140
@christophercurrie The outstanding item for now is a set of LICENSE/NOTICE
files for nifi-aws-service-api-nar, similar to what is now in the nifi-aws-nar.
I believe the NOTICE file can be pared down
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2140
@christophercurrie I think we're pretty close on this PR, any interest in
continuing?
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2140
More migration notes:
* My 1.3.0 AWS processor worked OK without modification when used with this
PR.
* My 1.3.0 AWS controller service did not work with just the NAR file
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2140
Thanks for the update, @christophercurrie . This PR is looking pretty good:
* Passes the full suite of unit tests with contrib-check.
* AWS processors and controller service still work OK in my
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2140
Thanks, @bbende, I'll take up the migration guidance for the wiki. As part
of reviewing this PR, I am building some sample processor and service projects
to reproduce the problems and check the fix
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2140
Please add the commits to this one. It usually helps reviewing to see the
commits separately, and it's easy enough to squash at the end.
---
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2140#discussion_r138521026
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/pom.xml ---
@@ -253,11 +253,6
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2140#discussion_r138520916
--- Diff: pom.xml ---
@@ -1109,12 +1109,6 @@
org.apache.nifi
-nifi-kudu-nar
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2140#discussion_r138520837
--- Diff: nifi-assembly/pom.xml ---
@@ -1,13 +1,13 @@
-
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2140
@christophercurrie - With respect to a transition plan, I'm not sure
exactly what we need. I'll have to get back to you on that. In vague concept,
users who have built custom processors and custom
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2140#discussion_r138251181
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/pom.xml ---
@@ -253,11 +253,6
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2140#discussion_r138248781
--- Diff: nifi-assembly/pom.xml ---
@@ -1,13 +1,13 @@
-
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2140#discussion_r138251087
--- Diff: pom.xml ---
@@ -1109,12 +1109,6 @@
org.apache.nifi
-nifi-kudu-nar
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2140
Reviewing... I agree the CI failure may not be related to this change.
`mvn clean install -Pcontrib-check` worked OK for me.
---
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2017#discussion_r137959726
--- Diff:
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/AbstractGCPProcessor.java
---
@@ -55,6 +55,25
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2017
@trixpan - Thanks for contributing the proxy feature. It looks pretty
good, and worked just fine in my testing with GCS processors. Please take a
look at the nit-picks above, but I don't have any
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2017#discussion_r137959785
--- Diff:
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/AbstractGCPProcessor.java
---
@@ -55,6 +55,25
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2017#discussion_r137959775
--- Diff:
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/AbstractGCPProcessor.java
---
@@ -55,6 +55,25
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2110
Thanks, @markap14, looks good.
---
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2110
Reviewing...
---
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
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2066
@baank Thanks for putting together this PR, it looks like you put a lot of
thought into covering all the possible encryption scenarios. I haven't run it
yet, but I have a few starter questions after
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2066
Reviewing...
---
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
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1888#discussion_r130516693
--- Diff:
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/test/java/org/apache/nifi/processors/aws/sqs/ITDeleteSQS.java
---
@@ -0,0 +1,83
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2034
Looks good. Thanks again for fixing this, @Wesley-Lawrence.
---
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
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2030
Thanks for fixing those items, @mans2singh, I'll get this merged.
---
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
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2034
@Wesley-Lawrence I think this is a good fix, your approach to the solution
seems solid. Thanks for adding the unit tests for the recursive and
mutually-referential cases. Would you please
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2034#discussion_r130225722
--- Diff:
nifi-nar-bundles/nifi-extension-utils/nifi-record-utils/nifi-avro-record-utils/src/main/java/org/apache/nifi/avro/AvroTypeUtil.java
---
@@ -218,6
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1126
@baank - Thanks for being willing to work on this. A new pull request
might be easier.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2034
Thanks @pvillard31!
@Wesley-Lawrence Don't worry about squashing yet, we can do that as a final
step.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2034
@Wesley-Lawrence Thanks for putting this PR together. I am still reviewing
- the only immediate feedback I can give is that I would prefer not to update
the checkstyle definition in pom.xml as part
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2034
Reviewing...
---
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
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2030
Overall, this looks good - tests pass, integration tests pass, refactoring
of integration test credentials is nice, code looks good. In my basic testing,
it deleted objects from RethinkDB just fine
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2030
Reviewing...
---
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
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2025
Thanks, @pvillard31, for taking the time to improve the documentation
formatting.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1972
@peter-gergely-horvath - Thanks for the update, it looks good. I will
merge shortly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2012
Thanks for those updates, @mans2singh, everything looks good. I'll merge
shortly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2012
I'm OK with the name "GetRethinkDB". Thanks for the tagging.
---
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 pr
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2012
My question about the not_found relationship is not if we should have it or
not -- I think it is a great addition. My question is about the logging for
that code path. You log an error on line 150
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2012#discussion_r128149561
--- Diff:
nifi-nar-bundles/nifi-rethinkdb-bundle/nifi-rethinkdb-processors/src/main/java/org/apache/nifi/processors/rethinkdb/GetRethinkDB.java
---
@@ -137,9
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2012#discussion_r127879683
--- Diff:
nifi-nar-bundles/nifi-rethinkdb-bundle/nifi-rethinkdb-processors/src/main/java/org/apache/nifi/processors/rethinkdb/GetRethinkDB.java
---
@@ -0,0
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2012#discussion_r127882745
--- Diff:
nifi-nar-bundles/nifi-rethinkdb-bundle/nifi-rethinkdb-processors/src/main/java/org/apache/nifi/processors/rethinkdb/GetRethinkDB.java
---
@@ -0,0
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2012#discussion_r127880492
--- Diff:
nifi-nar-bundles/nifi-rethinkdb-bundle/nifi-rethinkdb-processors/src/main/java/org/apache/nifi/processors/rethinkdb/GetRethinkDB.java
---
@@ -0,0
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2012#discussion_r127880998
--- Diff:
nifi-nar-bundles/nifi-rethinkdb-bundle/nifi-rethinkdb-processors/src/main/java/org/apache/nifi/processors/rethinkdb/GetRethinkDB.java
---
@@ -0,0
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2012#discussion_r127879552
--- Diff:
nifi-nar-bundles/nifi-rethinkdb-bundle/nifi-rethinkdb-processors/src/main/java/org/apache/nifi/processors/rethinkdb/GetRethinkDB.java
---
@@ -0,0
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/2012
Reviewing...
---
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
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1972#discussion_r127591766
--- Diff:
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java ---
@@ -135,8 +135,8 @@
private volatile Set<Future> loggingF
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1972
@peter-gergely-horvath Thank you for submitting this PR, improving
testability is always welcome. I will be happy to review.
---
If your project is set up for it, you can reply to this email
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1888
@brosander I added an update to keep the existing attribute name. Please
let me know if/when we need a squash and rebase.
---
If your project is set up for it, you can reply to this email and have
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1998
@mans2singh Looks good, thanks for taking the time to fix 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
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1998
Reviewing...
---
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
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1982
Thanks @trkurc, @m-hogue. I confirmed the whitespace-only changes in the
diffs, ran full build with contrib-check OK. I'll merge shortly.
---
If your project is set up for it, you can reply
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1942
Thanks @mans2singh! This has been merged 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
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1942#discussion_r126293212
--- Diff:
nifi-nar-bundles/nifi-rethinkdb-bundle/nifi-rethinkdb-processors/src/test/java/org/apache/nifi/processors/rethinkdb/ITPutRethinkDBTest.java
Github user jvwing commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1942#discussion_r126293188
--- Diff:
nifi-nar-bundles/nifi-rethinkdb-bundle/nifi-rethinkdb-processors/src/main/java/org/apache/nifi/processors/rethinkdb/PutRethinkDB.java
---
@@ -0,0
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1942
@mans2singh, thanks for contributing this! I have a few general comments
from a first pass:
* The notice information for RethinkDB should be included in the
nifi-assembly/NOTICE file
1 - 100 of 286 matches
Mail list logo
