You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Stanton Sievers <ss...@us.ibm.com> on 2011/06/28 22:17:14 UTC
Review Request: Common Container precludes synchronous rpc callbacks for
handlers registered via Container.rpcRegister
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/974/
-----------------------------------------------------------
Review request for shindig and Michael Hermanto.
Summary
-------
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);
// }
Diffs
-----
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1140140
Diff: https://reviews.apache.org/r/974/diff
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.
Thanks,
Stanton
Re: Review Request: Common Container precludes synchronous rpc callbacks
for handlers registered via Container.rpcRegister
Posted by Stanton Sievers <ss...@us.ibm.com>.
Mike, if there are no other questions or concerns here, can you commit
this?
Thanks,
-Stanton
From: Michael Hermanto <mh...@gmail.com>
To: Ryan Baxter <rb...@gmail.com>,
Cc: johnfargo@gmail.com, shindig <de...@shindig.apache.org>, 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 <rb...@gmail.com> 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/>
>
Re: Review Request: Common Container precludes synchronous rpc
callbacks for handlers registered via Container.rpcRegister
Posted by Michael Hermanto <mh...@gmail.com>.
LGTM
On Thu, Jun 30, 2011 at 3:22 PM, Ryan Baxter <rb...@gmail.com> 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/>
>
Re: Review Request: Common Container precludes synchronous rpc callbacks for
handlers registered via Container.rpcRegister
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/974/#review937
-----------------------------------------------------------
Ship it!
LGTM
- Ryan
On 2011-06-30 13:09:14, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/974/
> -----------------------------------------------------------
>
> (Updated 2011-06-30 13:09:14)
>
>
> Review request for shindig, johnfargo and Michael Hermanto.
>
>
> Summary
> -------
>
> 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);
> // }
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1140140
>
> Diff: https://reviews.apache.org/r/974/diff
>
>
> 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.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: Common Container precludes synchronous rpc callbacks for
handlers registered via Container.rpcRegister
Posted by Stanton Sievers <ss...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/974/
-----------------------------------------------------------
(Updated 2011-06-30 13:09:14.957078)
Review request for shindig, johnfargo and Michael Hermanto.
Summary
-------
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);
// }
Diffs
-----
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1140140
Diff: https://reviews.apache.org/r/974/diff
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.
Thanks,
Stanton