+1 No further issues.
Let's try to do a good job of watching the various discussion lists for our communities, looking for places where devs had issues making sense of the documentation or examples. I'm expecting to feed in a bunch of clarifying edits in the next iteration. The addition is kind of hefty, but it is part of the prototype phase and is (so far) uncontroversial. Given the lack of any objections on this thread, I'm comfortable accepting this draft at EOD (4:30 PM PDT) today. If anyone objects and would like more time, please speak up. From: opensocial-and-gadgets-s...@googlegroups.com [mailto:opensocial-and-gadgets-s...@googlegroups.com] On Behalf Of Lane LiaBraaten Sent: Friday, April 03, 2009 9:58 PM To: opensocial-and-gadgets-s...@googlegroups.com Cc: shindig-dev@incubator.apache.org Subject: [opensocial-and-gadgets-spec] Re: OS Lite (OSAPI) spec doc Here's the latest patch: http://codereview.appspot.com/33058 -Lane On Fri, Apr 3, 2009 at 1:32 PM, Scott Seely <sse...@myspace-inc.com> wrote: Line 603 | 611 | 624 | 639 | 666 : We should show a check for errors prior to investigating result.[value]. People cut and paste this code all the time. Error checking shouldn't be confusing in the spec. It could be as simple as adding if (!result.error){/*existing code*/} good suggestion...added Functions section (starting at 676): Each method that takes optional parameters should have an <xref> to where the optional parameters are defined. Also, do any examples show how to pass in the optional parameters (I didn't see any)? If not, please add examples. I kept the simple examples and added some that show more optional parameters Line 680: "Takes an optional. set" should be "Takes an optional set" (delete the period after optional) nice catch...fixed From: opensocial-and-gadgets-s...@googlegroups.com [mailto:opensocial-and-gadgets-s...@googlegroups.com] On Behalf Of Lane LiaBraaten Sent: Friday, April 03, 2009 1:16 PM To: opensocial-and-gadgets-s...@googlegroups.com Cc: shindig-dev@incubator.apache.org Subject: [opensocial-and-gadgets-spec] Re: OS Lite (OSAPI) spec doc Here's the next pass: http://codereview.appspot.com/33058 On Fri, Apr 3, 2009 at 1:04 PM, Scott Seely <sse...@myspace-inc.com> wrote: Yes to the eref. I'm a bit concerned about the lack of parity between the JSON-RPC and osapi for album, mediaitem, and messaging. Osapi and JSON-RPC are closely related. I'm fine with leaving these undefined until v.next. Please post when you are complete and I'll give this another pass. From: opensocial-and-gadgets-s...@googlegroups.com [mailto:opensocial-and-gadgets-s...@googlegroups.com] On Behalf Of Lane LiaBraaten Sent: Friday, April 03, 2009 11:52 AM To: opensocial-and-gadgets-s...@googlegroups.com Cc: shindig-dev@incubator.apache.org Subject: [opensocial-and-gadgets-spec] Re: OS Lite (OSAPI) spec doc Thanks for the review, Scott. I synced up with Bob to address the open issues on this thread. I'm working on an updated patch, but in the mean time, comments inline... -Lane On Fri, Apr 3, 2009 at 9:59 AM, Scott Seely <sse...@myspace-inc.com> wrote: Based on my review, this is still a work in progress. I do not guarantee that this is a complete list of spec issues. These are simply the issues that seem to be the most evident. Missing from spec: Messaging Albums MediaItem I think we should move these to v.next Line 588: "Highlights" is really a section. Please change from <t> to <section title="Highlights"> fixed Line 600: Please strike. This is a sentence that occurs in the discussion of why you included the examples early. It is not needed for the spec. removed Line 601: "A Person request, the most basic example" should be moved into the <figure> tag as: <figure> <preamble>A basic Person Request</preamble> Line 609 & 618: Move content inside /figure/preamble tag fixed here and in a couple other places as well Line 631 Move this to Highlights list: Batch calls can be cascaded. Maybe I'm old school, but it seems that the correct term is "chained|chaining"? Suggested edit when moving to Highlights (near line 588) "Batch calls are designed for call chaining." The examples demonstrate this concept adequately. Moved to highlights list and changed to 'chaining'. Line 635. Please strike "This is a short namespace, so it's easier to type, and it is different from the existing apis so that they can coexist until the old apis are removed." I don't believe that this statement enhances the spec. Line 586 already addresses deprecation. removed Line 648: "When the Gadget server renders the osapi library, it calls the SystemHandler's list Methods endpoint, and adds the available services." 'it' is ambiguous. SystemHandler is a new term. I believe that this may be a Shindig type, not something that is actually part of the system. Line 648 contains a TODO: "The js client then uses this to generate (TODO Not implemented yet)". The document here MUST NOT refer to TODOs in any given implementation. The document MUST NOT contain TODOs for feature descriptions. I'm not sure which case the TODO refers to, but the TODO needs to disappear and be rectified. removed implementation details (and TODO). The paragraph now reads: The features available on the client are determined by the server from which the JavaScript is delivered. The server SHOULD generate service methods for all available services. Developers wanting to write gadgets safely can test for the existence of a service and method before making their call. [example follows...] Line 661: Define behavior of <Require feature="osapi" /> Line 662 | 682 | 698 | 715 | 735 | 750 | 775 Why are we defining subsets of the feature? Osapi should be a single feature, introducing the basic APIs around Activities, People, AppData. The extra granularity seems to be new and unnecessary. I fail to see the benefit of the extra knobs. Let's just use feature="osapi" for v0.9. We can revisit sub-features later, but at this point it doesn't seem to provide much benefit. Line 662: "that map to the activities endpoint" should be "that map to the people endpoint". Need precise definition of what the endpoint is. Probably need a cross reference to the spec where the endpoint for each item is defined will do...this is an eref tag, right? Line 666 | 667 | 668 | 669: You repeat "convenience" 2x. Delete the 'Note' removed the duplication Line 682 | 698: Need precise definition of what the endpoint is. Probably need a cross reference to the spec where the endpoint for each item is defined will do. From: opensocial-and-gadgets-s...@googlegroups.com [mailto:opensocial-and-gadgets-s...@googlegroups.com] On Behalf Of Lane LiaBraaten Sent: Thursday, April 02, 2009 9:45 PM To: opensocial-and-gadgets-s...@googlegroups.com Cc: shindig-dev@incubator.apache.org Subject: [opensocial-and-gadgets-spec] Re: OS Lite (OSAPI) spec doc 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-Spe cification.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 I'm going to take osapi.base out of the spec for v0.9 - 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 will add central documentation about "Request objects" (which have execute() methods) and "Batch Request objects" (which have execute() and add() methods) - 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 goodbye osapi.ui - 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 Bob thinks he can get delete() to work...if not, we'll use remove() - " 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 see commnet above about documenting "Batch Request objects" - 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 Let's punt on this for now...I don't think many developers will use it - 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 I'm planning to create methods for head, get, put, post, delete - 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 I think we can get rid of the ui namespace. I agree that we should operate under the assumption that any js call can be ui-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 For more options, visit this group at http://groups.google.com/group/opensocial-and-gadgets-spec?hl=en -~----------~----~----~----~------~----~------~--~---
