You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Ryan J Baxter <rj...@us.ibm.com> on 2011/09/01 03:25:44 UTC
Re: Review Request: Allow for incremental preloading of gadget metadata and
security tokens
Mike, would you mind taking a quick look like this?
-Ryan
Email: rjbaxter@us.ibm.com
Phone: 978-899-3041
developerWorks Profile
From: "Jesse Ciancetta" <jc...@mitre.org>
To: "shindig" <de...@shindig.apache.org>, "Jesse Ciancetta"
<jc...@mitre.org>, "Ryan Baxter" <rb...@gmail.com>,
Date: 08/25/2011 11:31 AM
Subject: Re: Review Request: Allow for incremental preloading of
gadget metadata and security tokens
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1563/
-----------------------------------------------------------
(Updated 2011-08-25 15:32:43.908637)
Review request for shindig.
Changes
-------
Updates per Ryan's review.
Summary
-------
Common container currently supports a one-time bulk preload of gadget
metadata and security tokens via the opt_config parameter passed to the
Container constructor, however it would also be useful to have a way to
incrementally preload this data as well.
We're currently doing this in Apache Rave by calling the private
container.preloadFromConfig_ function for each of our gadgets before
calling container.navigateGadget which seems to work just fine, but we
obviously don't want to continue to rely on a private function.
I'm attaching a patch to make the preloadFromConfig_ function public -- 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 addresses bugs RAVE-172 and SHINDIG-1579.
https://issues.apache.org/jira/browse/RAVE-172
https://issues.apache.org/jira/browse/SHINDIG-1579
Diffs (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js
1157893
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/feature.xml
1137021
http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js
1086837
Diff: https://reviews.apache.org/r/1563/diff
Testing
-------
Additional unit tests added as well as testing in the browser using sample
common container.
Thanks,
Jesse
RE: Review Request: Allow for incremental preloading of gadget
metadata and security tokens
Posted by "Ciancetta, Jesse E." <jc...@mitre.org>.
*bump*
I've had a few common container related patches open on the review board for close to a month now -- could I trouble the Shindig committers to please help me see them through?
I (and the Apache Rave community who are waiting on these patches) would very much appreciate it!
Please see the previous message in this email thread for details on the pending reviews.
Thanks!
>-----Original Message-----
>From: Ciancetta, Jesse E.
>Sent: Thursday, September 01, 2011 9:09 AM
>To: 'Ryan J Baxter'; dev@shindig.apache.org; Michael Hermanto
>Subject: RE: Review Request: Allow for incremental preloading of gadget
>metadata and security tokens
>
>Thanks for following up on this Ryan.
>
>Mike -- there was some additional discussion around this patch on the dev@
>list -- here is a link to an archive of that discussion for your reference:
>
>http://markmail.org/message/awaz4hkcmb4yfboi
>
>I also have another fairly simple common container patch that I posted a
>couple of weeks ago to enable implementers to plug in their own token fetch
>function -- if you have a minute to take a look at that one too I'd appreciate it.
>The review for that one is here:
>
>https://reviews.apache.org/r/1564/
>
>with some additional discussion in the JIRA comments here:
>
>https://issues.apache.org/jira/browse/SHINDIG-1580
>
>And finally -- I have a third (much larger) review open for getting moduleId's
>into security tokens which (IMO) addresses a significant oversight/bug in the
>current common container implementation:
>
>https://reviews.apache.org/r/1632/
>
>If you (or anyone else who doesn't mind taking a look) could even just
>confirm that this is indeed an issue that should be resolved in the common
>container it might be helpful in kicking off some further discussion around the
>patch.
>
>Thanks!
>
>From: Ryan J Baxter [mailto:rjbaxter@us.ibm.com]
>Sent: Wednesday, August 31, 2011 9:26 PM
>To: dev@shindig.apache.org; Michael Hermanto
>Cc: Ciancetta, Jesse E.
>Subject: Re: Review Request: Allow for incremental preloading of gadget
>metadata and security tokens
>
>Mike, would you mind taking a quick look like this?
>
>-Ryan
>
>Email: rjbaxter@us.ibm.com
>Phone: 978-899-3041
>developerWorks Profile
>
>
>
>From: "Jesse Ciancetta" <jc...@mitre.org>
>To: "shindig" <de...@shindig.apache.org>, "Jesse Ciancetta"
><jc...@mitre.org>, "Ryan Baxter" <rb...@gmail.com>,
>Date: 08/25/2011 11:31 AM
>Subject: Re: Review Request: Allow for incremental preloading of gadget
>metadata and security tokens
>________________________________________
>
>
>
>
>-----------------------------------------------------------
>This is an automatically generated e-mail. To reply, visit:
>https://reviews.apache.org/r/1563/
>-----------------------------------------------------------
>
>(Updated 2011-08-25 15:32:43.908637)
>
>
>Review request for shindig.
>
>
>Changes
>-------
>
>Updates per Ryan's review.
>
>
>Summary
>-------
>
>Common container currently supports a one-time bulk preload of gadget
>metadata and security tokens via the opt_config parameter passed to the
>Container constructor, however it would also be useful to have a way to
>incrementally preload this data as well.
>
>We're currently doing this in Apache Rave by calling the private
>container.preloadFromConfig_ function for each of our gadgets before calling
>container.navigateGadget which seems to work just fine, but we obviously
>don't want to continue to rely on a private function.
>
>I'm attaching a patch to make the preloadFromConfig_ function public -- 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 addresses bugs RAVE-172 and SHINDIG-1579.
> https://issues.apache.org/jira/browse/RAVE-172
> https://issues.apache.org/jira/browse/SHINDIG-1579
>
>
>Diffs (updated)
>-----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript
>/features/container/container.js1157893
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript
>/features/container/feature.xml1137021
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/
>features/container/container_test.js1086837
>
>Diff: https://reviews.apache.org/r/1563/diff
>
>
>Testing
>-------
>
>Additional unit tests added as well as testing in the browser using sample
>common container.
>
>
>Thanks,
>
>Jesse
>
RE: Review Request: Allow for incremental preloading of gadget
metadata and security tokens
Posted by "Ciancetta, Jesse E." <jc...@mitre.org>.
Thanks for following up on this Ryan.
Mike -- there was some additional discussion around this patch on the dev@ list -- here is a link to an archive of that discussion for your reference:
http://markmail.org/message/awaz4hkcmb4yfboi
I also have another fairly simple common container patch that I posted a couple of weeks ago to enable implementers to plug in their own token fetch function -- if you have a minute to take a look at that one too I'd appreciate it. The review for that one is here:
https://reviews.apache.org/r/1564/
with some additional discussion in the JIRA comments here:
https://issues.apache.org/jira/browse/SHINDIG-1580
And finally -- I have a third (much larger) review open for getting moduleId's into security tokens which (IMO) addresses a significant oversight/bug in the current common container implementation:
https://reviews.apache.org/r/1632/
If you (or anyone else who doesn't mind taking a look) could even just confirm that this is indeed an issue that should be resolved in the common container it might be helpful in kicking off some further discussion around the patch.
Thanks!
From: Ryan J Baxter [mailto:rjbaxter@us.ibm.com]
Sent: Wednesday, August 31, 2011 9:26 PM
To: dev@shindig.apache.org; Michael Hermanto
Cc: Ciancetta, Jesse E.
Subject: Re: Review Request: Allow for incremental preloading of gadget metadata and security tokens
Mike, would you mind taking a quick look like this?
-Ryan
Email: rjbaxter@us.ibm.com
Phone: 978-899-3041
developerWorks Profile
From: "Jesse Ciancetta" <jc...@mitre.org>
To: "shindig" <de...@shindig.apache.org>, "Jesse Ciancetta" <jc...@mitre.org>, "Ryan Baxter" <rb...@gmail.com>,
Date: 08/25/2011 11:31 AM
Subject: Re: Review Request: Allow for incremental preloading of gadget metadata and security tokens
________________________________________
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1563/
-----------------------------------------------------------
(Updated 2011-08-25 15:32:43.908637)
Review request for shindig.
Changes
-------
Updates per Ryan's review.
Summary
-------
Common container currently supports a one-time bulk preload of gadget metadata and security tokens via the opt_config parameter passed to the Container constructor, however it would also be useful to have a way to incrementally preload this data as well.
We're currently doing this in Apache Rave by calling the private container.preloadFromConfig_ function for each of our gadgets before calling container.navigateGadget which seems to work just fine, but we obviously don't want to continue to rely on a private function.
I'm attaching a patch to make the preloadFromConfig_ function public -- 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 addresses bugs RAVE-172 and SHINDIG-1579.
https://issues.apache.org/jira/browse/RAVE-172
https://issues.apache.org/jira/browse/SHINDIG-1579
Diffs (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js1157893
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/feature.xml1137021
http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js1086837
Diff: https://reviews.apache.org/r/1563/diff
Testing
-------
Additional unit tests added as well as testing in the browser using sample common container.
Thanks,
Jesse