I created a spec patch for the OSAPI stuff, but I didn't realize Bob hadn't incorporated Adam's comments yet. I made a few changes, but some of these we still need to finalize. My comments inline.
-Lane On Tue, Mar 31, 2009 at 10:10 AM, Adam Winer <[email protected]> wrote: > > Comments: > > - At this stage, we really need spec updates in the form of diffs to > > http://opensocial-resources.googlecode.com/svn/spec/draft/Opensocial-Specification.xml > . > I kinda wish we'd split this huuuge doc into multiple smaller docs, > as the XSLT transform hangs Firefox for awhile, but that should be its > own spec diff. > > - osapi.base should largely be undocumented, and I'm not sure it > should exist as a feature name, though I could be convinced otherwise. no change ... I'm not clear on this so I don't have an opinion right now > > > - if (osapi.people.getViewer) { > should be > if (osapi.people && osapi.people.getViewer) { > since you have to test for both the service and the method. fixed > > > - "When no parameters are specified, the server defaults will be used: > { user : @me, group : @self}. This is built on > osapi.newJsonRequest()." > Two notes. It's userId: @me, groupId: @self. And what it's built > on is an implementation detail that the spec should leave out. fixed > > > - osapi.people.get.execute(callback) > isn't a great description, since there is no such function on > osapi.people.get. Need some central documentation describing the > execute() method on all request and batch objects no change - not sure what the change should be here > > > - osapi.activities.create(params) - maps to the activities create > endpoint. This is a non-ui intermediated version of the > osapi.ui.requestCreateActivity call. > I think the distinction between UI-intermediated calls and > non-UI-intermediated calls is artificial. A container may make *any* > JS call ui-mediated - permission dialogs being the obvious example. > Strongly prefer if we: > (1) Kill requestCreateActivity(), and for that matter everything in > osapi.ui > (2) Leave UI mediation up to the container, in particular use > activities.create() instead of requestCreateActivity() > (3) If we have time, build the message service API for the > requestSendMessage()/requestShareApp(), if not leave that for v.Next > (4) Don't bother with requestPermission() in this API, though that > opinion is 99% based on experience with a few containers, and other > container developers may have very different feedback. no change, but I'm +1 on these comments > > > - osapi.appdata.deleteData > Since JS prevents us from just calling this "delete", I'd rather > call it "remove()", which is at least a predictable naming pattern. > Or "deleteAppdata()". no change, but I'm +1 > > > - " osapi.messages TODO" > I think this means it's not in 0.9. removed > > > * osapi.newBatch() - returns a new osapi.base.batch request. > * osapi.base.batch.add(key, request) - Adds a service request to > the batch associated with the specified key. These may be jsonrpc > calls, such as osapi.people.get(), or osapi.makeRequest calls to third > parties, or a mixture of the two. > * osapi.base.batch.execute(callback) - executes all of the > requests in the batch. Callback is passed a JSON object mapping each > request key to a JSON response object. > > Don't doc that batches are in "osapi.base.batch". All we need to doc > is that batch objects have add() and execute() methods. no change, I think this makes sense though, esp if we take out the osapi.base docs > > > - Batch functions:" > > One thing I would recommend adding is tweaking osapi.newBatch() to be: > osapi.newBatch(opt_json) > ... where opt_json is a JSON array describing a full request. This > lets developers that are truly fluent with JSON-RPC write: > osapi.newBatch([{method: "people.get"} ... etc...]).execute(callback); > ... and done. no change, seems reasonable > > > > - osapi.makeRequest > This should be consistent with everything else in osapi, not with > gadgets.io. I think the right API is to follow Data Pipelining: > > osapi.http.get(params) > osapi.http.post(params) > > (and any other HTTP methods we care to specify at this point). > "params" should exactly match the attributes on os:HttpRequest, and > must use the same data format returned by os:HttpRequest (which I > haven't specified yet, I will send out that today). It also shouldn't > mention that it uses gadgets.io.makeRequest(). no change, but I like it > > > - osapi.ui: > > As said above, I think this needs to be killed, as ui-mediated is up > to the container, not the spec. Batching semantics get a little > tricky, but we're far better off supporting batching for ui-mediated > requests than not, because it gives the container a lot more > flexibility for good UIs - instead of showing 5 dialogs in succession, > the container can show a single dialog saying the gadget's trying to > do 5 things. For now, we can leave the batch semantics up to the > container, as I can come up with a variety of plausible semantics. > > Also, everything else is method(params).execute(callback), and there's > no reason to break down and use positional parameters here. no change - the ui namespace does seem strange since there's nothing that says the container *has* to make these calls user-mediated > > > - osapi.base: > > I don't think any of these need to be documented. If containers > offer additional service methods, it's up to containers to support > getting those methods into osapi. The SystemHandler method you > describe above is reasonable. no change. I don't fully understand this, but if containers don't need to be consistent on the osapi.base namespace, then we can leave this out > > > - Error Handling: > > I like the pattern of using and propagating "error" through the > structures. A big +1, as it's simple and clear. That said, the doc > needs to be explicit that: > - "error" may not be used as a key to batch.add(). > - "error" is reserved and may not be used as a property of a JSON > RPC batch response item. fixed > > Also, when the doc says "The error handling works exactly as it does > for the JSONRPC interface, since this api is based on that protocol." > that's not true. In JSON-RPC, a network error means that you get a > JSON structure with just an error: > {error: {message: "", code: ""}} > ... without any per-request-item content. What you're doing - which I > like - is creating dummy response items at each key and copying the > network error into each response item. no change - not sure how to update > > > In "Example: Processing any sub requests in the batch that succeeded: > ", the example is incorrect. We don't have an error property within > each returned activity, since all activities are in a single request. > We need an example like: > > var batch = osapi.newBatch(). > add('v', osapi.people.getViewer()). > add('vf', osapi.people.getViewerFriends()); > > batch.execute(function(result) { > for (var name in result) { > if (result.hasOwnProperty(name)) { > if (result[name].error) { > gadgets.log("Found an error: ", result[name].error.message); > } else { > gadgets.log("Got some data: ", result[name]); > } > } > } > }); fixed > > > > On Mon, Mar 30, 2009 at 12:15 PM, Bob Evans <[email protected]> wrote: > > > > The URL: http://wiki.opensocial.org/index.php?title=OSAPI_Specification > > > > On Mon, Mar 30, 2009 at 12:01 PM, Bob Evans <[email protected]> wrote: > >> Hi, > >> > >> I have updated the OS Lite spec doc to match the current > >> implementation (there is a follow on patch coming today with the > >> metadata-based discovery of service methods). > >> > >> Known issues: > >> > >> * requestCreateActivity and activities.create. It might be possible to > >> make all calls ui-interruptible, but this prevents batching, well, > >> it makes batching a lot stranger and more involved. For example, what > >> happens if I have a batch where 2 of 4 calls require UI interaction > >> with the user. It's do-able, but would require a lot more thought than > >> .9 allows, I think. My proposed solution keep requestX methods, and > >> discuss making all service calls ui-intermediable in the next version > >> spec discussion. > >> > >> * send vs execute vs call naming. I prefer execute, but if people want > >> a shorter name, then send seems good. Some have suggested call as a > >> name, but this is used in the language, so seems like a bad idea. > >> > >> * Rename osapi.makeRequest to osapi.http.get/post. To me, makeRequest > >> reflects the existing, underlying gadgets calls, but I could see > >> making it http. > >> > >> * osapi.ui namespace is weird, better suggestions? > >> > >> * Should we have a bucket of params for the requestX calls, or > >> positional params as it is now? Maybe all of this is obviated if we > >> make the requestX calls be jsonrpc calls. > >> > >> > >> Thanks, > >> Bob > >> > > > > > > > > > --~--~---------~--~----~------------~-------~--~----~ > You received this message because you are subscribed to the Google Groups > "OpenSocial and Gadgets Specification Discussion" group. > To post to this group, send email to > [email protected] > To unsubscribe from this group, send email to > [email protected]<opensocial-and-gadgets-spec%[email protected]> > For more options, visit this group at > http://groups.google.com/group/opensocial-and-gadgets-spec?hl=en > -~----------~----~----~----~------~----~------~--~--- > >
