nifi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tony Kurc <trk...@gmail.com>
Subject Re: PRs
Date Thu, 26 Nov 2015 17:31:04 GMT
I created NIFI-1225 which are changes to S3 processors without Multipart.
Submitted a patch for review. Joe Skora, would you have a chance to look it
over?

On Thu, Nov 26, 2015 at 12:41 AM, Tony Kurc <trkurc@gmail.com> wrote:

> 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.witt@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 <trkurc@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 <trkurc@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.witt@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 <trkurc@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.gilman@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.witt@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 <trkurc@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 <
>> aldrinpiri@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
<trkurc@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
>> >>> >> >>>
>> >>> >>
>> >>>
>> >>
>> >>
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message