Understood tony - thanks for digging into the review so thoroughly and Joe thank you. This is a very non-trivial contrib.
On Thu, Nov 26, 2015 at 12:12 AM, Tony Kurc <trk...@gmail.com> wrote: > I recommend we push NIFI-1107 to next release. We discovered some unfun > issues the S3 Multipart "API" creates, notably, leaving dangling pieces > around [1]: > > "Once you initiate a multipart upload there is no expiry; you must > explicitly complete or abort the multipart upload" > > And charging while they're there [2]: > > "After you initiate multipart upload and upload one or more parts, you must > either complete or abort multipart upload in order to stop getting charged > for storage of the uploaded parts. Only after you either complete or abort > multipart upload, Amazon S3 frees up the parts storage and stops charging > you for the parts storage." > > This, plus grappling with persisting state for the multipart (and lack of > framework support for persistent state) means I think we have some more > work to do despite the feature being in a "works" state. > > [1] http://docs.aws.amazon.com/AmazonS3/latest/dev/uploadobjusingmpu.html > [2] http://docs.aws.amazon.com/AmazonS3/latest/dev/mpuoverview.html > > On Wed, Nov 25, 2015 at 7:16 PM, Tony Kurc <trk...@gmail.com> wrote: > >> Things that make me feel better: The persistence mechanism is very similar >> to that of ListHDFS. >> >> >> https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-hadoop-bundle/nifi-hdfs-processors/src/main/java/org/apache/nifi/processors/hadoop/ListHDFS.java#L417 >> >> On Tue, Nov 24, 2015 at 10:56 PM, Joe Witt <joe.w...@gmail.com> wrote: >> >>> Tags are a great place to mark experimental. We used to plan for this >>> concept outright and make it look at scary and such on the ui. But >>> folks just didn't care. They used it anyway. Happy to revisit it but >>> for now perhaps just adding a tag of experimental is enough. >>> >>> If the existing code path is largely untouched then that is certainly >>> great for moving the ball forward. In fairness to Joe S or anyone >>> that has to persist internal process state until we offer that as part >>> of the framework it is much harder than we want it to be for people. >>> Will take a look through the state methods but Payne is probably the >>> best at playing the wack a mole edge case game for such things. >>> >>> On Tue, Nov 24, 2015 at 10:52 PM, Tony Kurc <trk...@gmail.com> wrote: >>> > So, I beat on the the patch for NIFI-1107, and as I suspected, it is >>> > awfully low risk for existing flows, but I think I'd need a second >>> opinion >>> > on how state is kept for resuming uploads. I believe it will work, and >>> it >>> > looks like a lot of the edge cases are covered if somehow state is lost >>> or >>> > corrupted, but I'm not sure if I am comfortable with how it fits >>> > architecturally. If someone has cycles, and can peruse the *State >>> methods >>> > (getState, persistState, ...) and weigh in, it would accelerate my >>> review >>> > significantly. >>> > >>> > Also, it sure would be great to mark features as experimental! >>> > >>> > >>> > >>> > On Tue, Nov 24, 2015 at 10:36 PM, Matt Gilman <matt.c.gil...@gmail.com> >>> > wrote: >>> > >>> >> These tickets [1][2] address the incorrect validation errors we were >>> >> seeing for processors that include the Input Required annotation. These >>> >> were bugs that slipped through the NIFI-810 the review. Would be good >>> to >>> >> include if possible but I understand we need to draw the line >>> somewhere. >>> >> >>> >> As for NIFI-655, I've been struggling getting an LDAP server stood up >>> that >>> >> uses 2 way SSL. Hopefully we can get that squared away soon and wrap >>> this >>> >> one up. :) >>> >> >>> >> Matt >>> >> >>> >> [1] https://issues.apache.org/jira/browse/NIFI-1198 >>> >> [2] https://issues.apache.org/jira/browse/NIFI-1203 >>> >> >>> >> Sent from my iPhone >>> >> >>> >> > On Nov 24, 2015, at 10:23 PM, Joe Witt <joe.w...@gmail.com> wrote: >>> >> > >>> >> > Given the testing to NIFI-1192 and review of NIFI-631 done already >>> >> > both are lower risk I think. >>> >> > >>> >> > NIFI-1107 seems very useful and helpful but we do need to be careful >>> >> > given that we know this one is already in use and this is a >>> >> > substantive change. >>> >> > >>> >> > If there are folks that can dig into review/testing of NIFI-1107 that >>> >> > would be great. Waiting for word on NIFI-655 readiness then I think >>> >> > we should go cold and just focus on testing an RC. >>> >> > >>> >> > Thanks >>> >> > Joe >>> >> > >>> >> >> On Tue, Nov 24, 2015 at 4:22 PM, Tony Kurc <trk...@gmail.com> >>> wrote: >>> >> >> Agreed. I know there has already been a good deal of discussion >>> about >>> >> >> design on all these. >>> >> >> >>> >> >>> On Tue, Nov 24, 2015 at 4:14 PM, Aldrin Piri <aldrinp...@gmail.com >>> > >>> >> wrote: >>> >> >>> >>> >> >>> No qualms here. If they look good to go while the work and testing >>> >> >>> surrounding NIFI-655 wraps up, they might as well be included. >>> Would >>> >> not >>> >> >>> want to delay the release should any of these become protracted in >>> >> terms of >>> >> >>> iterations. >>> >> >>> >>> >> >>>> On Tue, Nov 24, 2015 at 4:05 PM, Tony Kurc <trk...@gmail.com> >>> wrote: >>> >> >>>> >>> >> >>>> All, >>> >> >>>> I was reviewing github PRs and wondering whether anyone objected >>> to >>> >> >>>> slipping a couple that look like they're very close into 0.4.0. >>> >> >>>> >>> >> >>>> NIFI-1192 (#131) >>> >> >>>> NIFI-631 (#113) >>> >> >>>> NIFI-1107 (#192) >>> >> >>>> >>> >> >>>> I should have some review cycles tonight. Lots of comments on them >>> >> all, >>> >> >>> and >>> >> >>>> have good "momentum". >>> >> >>>> >>> >> >>>> Tony >>> >> >>> >>> >> >>> >> >>
