Re: Review Request: Navigating to URLs in the common container has no indication of success or failure

2011-12-06 Thread Ryan Baxter
> On 2011-12-06 23:03:02, Stanton Sievers wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js, > > line 934 > > > > > > I'd rather be checking for a statu

Re: Review Request: Navigating to URLs in the common container has no indication of success or failure

2011-12-06 Thread Henry Saputra
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3037/#review3687 --- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javas

Re: Review Request: Navigating to URLs in the common container has no indication of success or failure

2011-12-06 Thread Ryan Baxter
> On 2011-12-06 23:03:02, Stanton Sievers wrote: > > Overall this seems OK. > > > > Have you looked at the performance impact here using lifecycle logging? > > > > The extra HTTP round trip could be costly, especially none of the HEAD > > request can be cached and reused by the subsequent

Re: Review Request: Navigating to URLs in the common container has no indication of success or failure

2011-12-06 Thread Ryan Baxter
> On 2011-12-06 23:15:05, Henry Saputra wrote: > > I am not too sure about extra http request to check for existence for URL. > > Can we just check for status code? Henry, what request would you check the status code of? - Ryan --- Thi

Re: OAUTH2 ClientAuthenticationHandler: No access to security token

2011-12-06 Thread daviesd
I see where my misunderstanding was. I thought the ClientAuthenticationHandler was called to set authentication values before the authorization code request, based on this link (it¹s misleading). http://docs.opensocial.org/display/OSD/Client+Authentication > "Allows shindig developers to inject

Re: Review Request: Navigating to URLs in the common container has no indication of success or failure

2011-12-06 Thread Henry Saputra
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3037/#review3683 --- I am not too sure about extra http request to check for existence for

Re: Review Request: Navigating to URLs in the common container has no indication of success or failure

2011-12-06 Thread Stanton Sievers
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3037/#review3681 --- Overall this seems OK. Have you looked at the performance impact he

Review Request: Navigating to URLs in the common container has no indication of success or failure

2011-12-06 Thread Ryan Baxter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3037/ --- Review request for shindig, Dan Dumont and Stanton Sievers. Summary --- Whe

OAUTH2 ClientAuthenticationHandler: No access to security token

2011-12-06 Thread daviesd
Argh! Not so great. The security token seems to be null even though I see the st param on the makeRequest call. I don¹t see any way inside of my ClientAuthenticationHandler to get at the original request object or the security token. This presents a bit of a problem for me, since we need to pas

Re: Adding client authentication handler

2011-12-06 Thread Li Xu
> > Yeah, that's a good point. It would be nice to define an extension as a > Set here to allows Multibinder to define > multiple binding. >

Re: Adding client authentication handler

2011-12-06 Thread daviesd
Li, thanks for the response. Ya, it's just unfortunate I have to overwrite the entire class. I tried to extend the class and overwrite the method, but since it's static that isn't going to work (guice complains about it already being bound). I noticed on this link http://docs.opensocial.org/dis

Re: Adding client authentication handler

2011-12-06 Thread Li Xu
Hi, Doug To my understanding, yes, it need to overwrite OAuth2HandlerModule to provide the new AuthenticationHandler list. Thanks for the catch about typo, will fix it. thanks, li On Tue, Dec 6, 2011 at 10:02 AM, daviesd wrote: > I didn¹t notice that the request coming into addOAuth2Authenticat

Re: Review Request: Actions, selection, and open-views feature issues in a locked domain environment.

2011-12-06 Thread Dan Dumont
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2994/#review3671 --- Ship it! Committed r1211090 - Dan On 2011-12-06 20:21:36, Dan Dumo

Re: Review Request: Actions, selection, and open-views feature issues in a locked domain environment.

2011-12-06 Thread Dan Dumont
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2994/ --- (Updated 2011-12-06 20:21:36.585266) Review request for shindig, Ryan Baxter, Ja

Re: Review Request: Actions, selection, and open-views feature issues in a locked domain environment.

2011-12-06 Thread Matthew Hatem
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2994/#review3668 --- Ship it! Looks good to me. - Matthew On 2011-12-06 17:54:35, Dan D

Re: Review Request: Actions, selection, and open-views feature issues in a locked domain environment.

2011-12-06 Thread Dan Dumont
> On 2011-12-04 18:55:14, Ryan Baxter wrote: > > You might want to test all your feature xml uses changes work when RPC > > arbitration is enabled as well. Turned that on and I don't see any issues running through the test gadgets. - Dan -

Re: Review Request: Actions, selection, and open-views feature issues in a locked domain environment.

2011-12-06 Thread Dan Dumont
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2994/ --- (Updated 2011-12-06 17:54:35.648638) Review request for shindig, Ryan Baxter, Ja

Re: Review Request: Metadata request may not return an array of gadget URLs when there is an error it may just return an error code and message need to account for that in EE code

2011-12-06 Thread Ryan Baxter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3005/#review3662 --- Committed revision 1210961. - Ryan On 2011-12-06 14:20:46, Ryan Bax

Re: Adding client authentication handler

2011-12-06 Thread daviesd
I didn¹t notice that the request coming into addOAuth2Authentication wasn¹t a HttpServletRequest but instead a org.apache.shindig.gadgets.http.HttpRequest with a securityToken accessor. Great! One more question if I might... What is the best way to add another authentication handler? Right now I

Re: Review Request: Metadata request may not return an array of gadget URLs when there is an error it may just return an error code and message need to account for that in EE code

2011-12-06 Thread Dan Dumont
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3005/#review3661 --- Ship it! LGTM - Dan On 2011-12-06 14:20:46, Ryan Baxter wrote: >

Re: Review Request: ConcatProxyServlet sets the HTTP response status after writing the response

2011-12-06 Thread Ryan Baxter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3006/#review3660 --- Committed revision 1210933. - Ryan On 2011-12-04 21:04:16, Ryan Bax

Re: Review Request: Metadata request may not return an array of gadget URLs when there is an error it may just return an error code and message need to account for that in EE code

2011-12-06 Thread Ryan Baxter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3005/ --- (Updated 2011-12-06 14:20:46.695618) Review request for shindig and Stanton Siev

Re: Review Request: Actions, selection, and open-views feature issues in a locked domain environment.

2011-12-06 Thread Dan Dumont
> On 2011-12-06 02:54:06, Ryan Baxter wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js, > > line 610 > > > > > > This line seems to be returning undefi

Re: Review Request: ConcatProxyServlet sets the HTTP response status after writing the response

2011-12-06 Thread Jesse Ciancetta
> On 2011-12-05 19:15:25, Brian Lillie wrote: > > In doFetchConcatResources, if one of the 2nd through nth requests > > encounters an error such that false would be returned, we still have the > > content from the 1st up to the failing request in the > > VerbatimConcatOutputStream, and when we