I'll second what joe said. non-trivial and a less than ideal API to work with. seriously?! no expiry!
On Thu, Nov 26, 2015 at 12:17 AM, Joe Witt <joe.w...@gmail.com> wrote: > 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 > >>> >> >>> > >>> >> > >>> > >> > >> >
