oops - here's the patch: http://codereview.appspot.com/33058
On Thu, Apr 2, 2009 at 9:42 PM, Lane LiaBraaten <lliab...@google.com> wrote: > 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 <awi...@gmail.com> 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 <bobev...@google.com> wrote: >> > >> > The URL: >> http://wiki.opensocial.org/index.php?title=OSAPI_Specification >> > >> > On Mon, Mar 30, 2009 at 12:01 PM, Bob Evans <bobev...@google.com> >> 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 >> opensocial-and-gadgets-s...@googlegroups.com >> To unsubscribe from this group, send email to >> opensocial-and-gadgets-spec+unsubscr...@googlegroups.com<opensocial-and-gadgets-spec%2bunsubscr...@googlegroups.com> >> For more options, visit this group at >> http://groups.google.com/group/opensocial-and-gadgets-spec?hl=en >> -~----------~----~----~----~------~----~------~--~--- >> >> >
