Hi Derell (and of course everybody following the discussion), sorry for my late answer, I've just returned from vacation today.
What a lively discussion! This morning, I read through the comments and the code you attached. First of all, many thanks for pushing RPC and your efforts to bring it in line with the new IO stack. In general, I agree that the implementation should be as close to the standard (version 2) as possible and not add too much logic. This should be understood as a general guideline, but not a strict principle (should, not must in RFC terms). I'm okay with the replaceFunction/reviverFunction because I can see that leaving it out would considerably limit both functionality and compatibility. About "qx1" requests, I'd favor to drop support and keep the new implementation clean (I suggest we keep the old implementation based on the old IO stack around as a contribution). The code you attached makes a pretty clean impression already, but dropping qx1 support it would really make it shine in my opinion. I like the idea to inject a preconfigured request, because – as you already pointed out – it makes RPC transport agnostic and from my understanding this seems to be one of the key ideas of Json-RPC in general. What seems a little counter-intuitive is the fact that the endpoint is specified on the IO level. However, I doubt that adding another parameter to the Rpc constructor is a good idea as this would involve repeating the URL for each call. What do you think? Regarding error handling, I tend to follow Gian Marco in that a "success" event should be linked to a response with the error property being null (and an "error" event vice-versa). I think the idea is to keep detection of the error condition away from the application code and this way make the application code code more readable. "success" is not determined by the transport, but rather the type of response (after all, a 200 OK does not mean success). "fail" should be reserved for transport specific and therefore "fatal" errors, as implemented (am I correct in assuming that a JSON-RPC service should actually never return a erroneous status code under normal conditions?). I see rpc.Coalesce fires and "error" event but perhaps this can be moved to rpc.Rpc. The specification does not seem to say anything about events, so I don't consider it a violation of the specification? I'm not sure about the events being simple data events with response and error as property. In rest.Resource we use events inheriting from the data event with additional properties. My argument is not necessarily that this approach is better, but rather that we should try to stick to one style of exposing data with fixed structure via events within IO related classes. About integrating the new RPC implementation into the framework, I believe it is a useful addition (and an improvement compared to the old implementation). Are you planing to provide unit tests? So far, we managed to maintain a good test coverage for all IO things and I would really like to maintain this level. Regards Tristan > Tristan and other interested in RPC, > > It's soooooo nice having a clean transport interface. We can do an RPC layer > that sits properly on top of transport! I spent some time working on an > implementation of RPC on top of the new transport. Since I won't be at a > computer on which I can work on this for the next week, I figured I'd post > this complete, but totally untested class, for comments. > > I was using rest.Resource as a model, although this one is simplified > further, since there is no reason at all for this class to have substantial > interaction with the transport request object. In this implementation, the > request object is provided to the Rpc class, fully configured, with the only > requirement that the request object implement qx.io.request.AbstractRequest, > and therefore provides a setRequestData() method and a send() method. > > The entire area of authentication, and out-of-band data which took up > substantial amounts of code in the old Rpc implementation is now left > completely to the caller to implement in whatever way is deemed appropriate. > I envision that request headers, parameters, etc. might be used for that > purpose. Since JSON-RPC is transport-agnostic, though, I wanted to leave all > of those aspects out of this class. > > Thoughts? > > See attached. > > Derrell > > <Rpc.js>------------------------------------------------------------------------------ > Get a FREE DOWNLOAD! and learn more about uberSVN rich system, > user administration capabilities and model configuration. Take > the hassle out of deploying and managing Subversion and the > tools developers use with it. > http://p.sf.net/sfu/wandisco-d2d-2_______________________________________________ > qooxdoo-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/qooxdoo-devel ------------------------------------------------------------------------------ EMC VNX: the world's simplest storage, starting under $10K The only unified storage solution that offers unified management Up to 160% more powerful than alternatives and 25% more efficient. Guaranteed. http://p.sf.net/sfu/emc-vnx-dev2dev _______________________________________________ qooxdoo-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/qooxdoo-devel
