thanks Ian for taking care of including all the provided feedback, I finally managed to review the patches and it looks very good now as it sounds simpler and more inline with the overall Oak design.
Thanks again, Tommaso Il giorno gio 7 set 2017 alle ore 16:39 Ian Boston <[email protected]> ha scritto: > On 7 September 2017 at 14:41, Francesco Mari <[email protected]> > wrote: > > > On Thu, Sep 7, 2017 at 11:05 AM, Ian Boston <[email protected]> wrote: > > > On 7 September 2017 at 07:22, Ian Boston <[email protected]> wrote: > > > > > >> Hi, > > >> > > >> On 6 September 2017 at 22:43, Michael Dürig <[email protected]> > wrote: > > >> > > >>> > > >>> > > >>> On 06.09.17 23:08, Michael Dürig wrote: > > >>> > > >>>> > > >>>> Hi, > > >>>> > > >>>> On 05.09.17 14:09, Ian Boston wrote: > > >>>> > > >>>>> Repeating the comment to on OAK-6575 here for further discussion. 2 > > new > > >>>>> Patches exploring both options. > > >>>>> > > >>>> > > >>>> I would actually prefer the original patch ( > > >>>> https://github.com/ieb/jackrabbit-oak/compare/trunk...ieb:O > > >>>> AK-6575?expand=1) in most parts. However I have concerns regarding > the > > >>>> generality of the new OakConversionService API as mentioned in my > > previous > > >>>> mail. I would be more comfortable if this could be restricted to > > something > > >>>> that resembles more like a "URIProvider", which given a blob returns > > an URI. > > >>>> > > >>>> On the implementation side, why do we need to introduce the > adaptable > > >>>> machinery? Couldn't we re-use the Whiteboard and OSGiWhiteBoard > > mechanisms > > >>>> instead? I think these could be used to track URIProvider instances > > >>>> registered by the various blob stores. > > >>>> > > >>>> > > >>> See https://github.com/mduerig/jackrabbit-oak/commit/2709c097b01 > > >>> a006784b7011135efcbbe3ce1ba88 for a *really* quickly hacked together > > and > > >>> entirely untested POC. But it should get the idea across though. > > >> > > >> > > >> > > >> Thank you. > > >> That makes sense. > > >> I think it only needs the java/org/apache/jackrabbit/ > > >> oak/blob/cloud/aws/s3/CloudFrontS3SignedUrlAdapterFactory.java and the > > >> API to be inside Oak, everything else can be in Sling. > > >> I'll update my patch and do a 2 options for Sling. > > >> > > > > > > > > > > > > > > > https://github.com/ieb/jackrabbit-oak/compare/trunk.. > > .ieb:OAK-6575-3?expand=1 > > > > > > and > > > > > > > https://github.com/apache/sling/compare/trunk...ieb:OAK-6575-3?expand=1 > > > > > > wdyt ? > > > > I like this a lot. It keeps Oak's side simple and cleanly integrates > > Oak's lower-level services in Sling. > > > > > Good news. > I think we should hold off committing the patch until Monday or Tuesday to > give those who may be offline this week a chance to comment. In particular > I have not seen a comment from Angela who I would expect to have a view as > this is acl/security related. That is assuming she is back online next > week. > > Best Regards > Ian > > > > > > > Obviously the second patch needs to be discussed with Sling dev, but is > > > should not be too contentious. > > > > > > Best Regards > > > Ian > > > > > > > > > > > >> > > >> I think that should address others concerns since it drops all signs > of > > >> any generic object to object conversion from Oak (Francesco), and > > doesn't > > >> require wide scale fragile changes with implied requirements being > > placed > > >> on how intermediate classes are connected and behave (mine). > > >> > > >> Best Regards > > >> Ian > > >> > > >> > > >>> > > >>> Michael > > >>> > > >> > > >> > > >
