Thanks for your comments Ryan -- responses inline. >RE 1563 >I have no problem with the changes, I just remember Mike having some >concerns.
I'm not sure that I'd agree that Mike had any real concerns -- I think he just didn't fully understand our use case and I replied to his message with some clarifications. Here is the link to the thread which has that discussion: http://groups.google.com/group/opensocial-and-gadgets-spec/browse_thread/thread/681c12ac2314facb For some reason though Mike's message doesn't show up in the thread, but my response to him is there which quotes his original message. If he does still have concerns however he should (please) voice them -- although since a month has passed since I replied to him and he hasn't commented further I'd like to assume "lazy consensus" from him and move on. It's probably also worth pointing out that there are +1's for this change from Paul L. and Mark W. in that thread as well. >I understand your use case but just a picky question for you. >Why do you load the common container before knowing if you even have any >"widgets" that require it? I think the only reason we're doing it the way we are is because it keeps our rendering process simple and consistent -- we have a simple array of mixed widgets which we loop over and delegate rendering for each one off to widget-type specific renderers. We could probably move to a two dimensional array of widgets instead first keyed by widget type -- in which case we'd have all of the OpenSocial type widgets in one array -- and at that point we'd be able to check to see if it's empty -- and if so skip common container entirely like you're suggesting -- but that still doesn't help us get all of the OpenSocial gadget metadata bulked up and ready for the one time preload in the call to the common container constructor -- we'd still have to loop over once to gather all the metadata and then again to call navigate for each one. Another use case I envision for incremental loading is if we decide to dynamically drop another OpenSocial gadget onto the page -- in that case I'm going to want to load the metadata for it with an XHR to Rave instead of Shindig (since our Rave metadata provider includes things like the user prefs for the gadget and other Rave specific data) -- but if I can't incrementally load that data into an existing common container instance, when I navigate that gadget, common container is going to make another call back to Shindig to fetch the metadata I already pulled from Rave -- and if that XHR is cross domain it won't work at all. >Could you wait to load the common container >until you have the list of gadgets which you need to render, then create >the common container and preload the metadata to populate the cache? Yes -- we can definitely change our rendering process in one way or another to work within the confines of the existing API, but I'd really rather not -- and I don't see any real reasons why we should have to. I think the change I'm proposing here is a safe change with no negative impact on existing clients, has received support from others in the community, and adds a little more flexibility in the way we use the common container. I think the bottom line is that we've provided at least two valid use cases for this change, and it's received 3 +1's from the community (including mine) -- so if there isn't a compelling reason *not* to do this, I personally think we should move forward with the change. Does anyone disagree? >-Ryan > >Email: rjbax...@us.ibm.com >Phone: 978-899-3041 >developerWorks Profile > > > >From: "Ciancetta, Jesse E." <jc...@mitre.org> >To: "dev@shindig.apache.org" <dev@shindig.apache.org>, >Date: 09/14/2011 01:48 PM >Subject: RE: [jira] [Resolved] (SHINDIG-1580) Allow for custom >security token fetch function to be used when refreshing security tokens > > > >Thanks Mat and Henry for pushing this patch through! > >I've got one other fairly simple review open which will also result in a >common container spec change, so once that one is done as well I'll go >ahead and open ticket for spec changes for both of them. > >The review for the other change is here: > >https://reviews.apache.org/r/1563/ > >There has already been additional discussion for that one and I think Ryan >was ready to commit it pending a quick review from Mike Hermanto, however >Mike never responded. > >Is there someone else who can help push that one through too? > >Thanks! > >>-----Original Message----- >>From: Henry Saputra (JIRA) [mailto:j...@apache.org] >>Sent: Tuesday, September 13, 2011 8:20 PM >>To: Ciancetta, Jesse E. >>Subject: [jira] [Resolved] (SHINDIG-1580) Allow for custom security token >>fetch function to be used when refreshing security tokens >> >> >> [ https://issues.apache.org/jira/browse/SHINDIG- >>1580?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] >> >>Henry Saputra resolved SHINDIG-1580. >>------------------------------------ >> >> Resolution: Fixed >> >>Closed with rev #1170371 >> >>> Allow for custom security token fetch function to be used when >refreshing >>security tokens >>> >----------------------------------------------------------------------------------------- >>> >>> Key: SHINDIG-1580 >>> URL: https://issues.apache.org/jira/browse/SHINDIG-1580 >>> Project: Shindig >>> Issue Type: Improvement >>> Reporter: Jesse Ciancetta >>> Fix For: 3.0.0 >>> >>> Attachments: gadget-token.patch >>> >>> >>> Currently the common container is hard coded to make a gadgets.token >>OSAPI call back to Shindig when refreshing security tokens. >>> I'm attaching a patch which allows users of the common container to >>override that default behavior by providing a custom function to use >instead. >>I've followed the same pattern which is already in place for the >>GET_PREFERENCES override for providing a custom function for fetching >>gadget preferences. >>> If this looks good to everyone I would be happy to go ahead and submit >a >>patch against the container specification for this change as well. >> >>-- >>This message is automatically generated by JIRA. >>For more information on JIRA, see: http://www.atlassian.com/software/jira >> >> > >
