Mike, if there are no other questions or concerns here, can you commit this?
Thanks, -Stanton From: Michael Hermanto <[email protected]> To: Ryan Baxter <[email protected]>, Cc: [email protected], shindig <[email protected]>, Stanton Sievers/Westford/IBM@Lotus Date: 07/06/2011 13:47 Subject: Re: Review Request: Common Container precludes synchronous rpc callbacks for handlers registered via Container.rpcRegister LGTM On Thu, Jun 30, 2011 at 3:22 PM, Ryan Baxter <[email protected]> wrote: > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/974/ > > Ship it! > > LGTM > > > - Ryan > > On June 30th, 2011, 1:09 p.m., Stanton Sievers wrote: > Review request for shindig, johnfargo and Michael Hermanto. > By Stanton Sievers. > > *Updated 2011-06-30 13:09:14* > Description > > This change is to accommodate synchronous rpc handlers that may be registered with the common container via osapi.container.Container.prototype.rpcRegister. Per the documentation in rpc.js#process, an rpc handler should return a result value that will be included as part of a synchronous callback made by the rpc service. If no result is provided it is assumed that the rpc handler will perform the callback. > > Currently rpcRegister in the common container does not allow for the returning of any results, thus forcing the rpc handler to perform the callback even in synchronous cases. > > For convenience, here is the documentation in rpc.js#process: > // If there is a callback for this service, attach a callback function > // to the rpc context object for asynchronous rpc services. > // > // Synchronous rpc request handlers should simply ignore it and return a > // value as usual. > // Asynchronous rpc request handlers, on the other hand, should pass its > // result to this callback function and not return a value on exit. > // > // For example, the following rpc handler passes the first parameter back > // to its rpc client with a one-second delay. > // > // function asyncRpcHandler(param) { > // var me = this; > // setTimeout(function() { > // me.callback(param); > // }, 1000); > // } > > Testing > > Minimal testing performed. We noticed this problem during some feature development in which a gadgets.rpc.call to the feature required a callback and the rpc handler in the container did not explicitly invoke the callback. This change fixed that problem. > > Diffs > > - > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > (1140140) > > View Diff <https://reviews.apache.org/r/974/diff/> >
