Graph API in batch mode does not accept post-processing block #237

Closed
wolframarnold opened this Issue Jul 18, 2012 · 3 comments

Projects

None yet

2 participants

@wolframarnold

I've got several batched Graph API requests, each of which I want to pass a block to for "post-processing" of the received data:

batch_client.get_connections "me", "photos"  { |resp| self.photos = resp }
batch_client.get_connections "me", "statuses" { |resp| self.statuses = resp }

This gives me a cleaner way to define what happens with the response than having to parse through a big array that's otherwise returned by the batch operation.

While the BatchOperation class accepts a block for exactly this purpose, the Graph API methods like get_object and get_connection do not accept such a block and don't pass it on. These methods call graph_call which, in batch mode, is switched to graph_call_in_batch with meta-programming trickery (aliasing). It seems that the post processing feature was not "finished" in this regard, and I can't find tests that the block is actually called.

Workaround

I can workaround this for now by making calls like to the graph batch call API directly:

batch_client.graph_call_in_batch("#{user}/#{connection_name}", {}, 'get', {}, &block)

API Proposal

I'd like to propose to change the API such that each of the methods in GraphAPIMethods accept a block and that this block, if present, is called with the result data. That would augment the standard (non-batch) API in a backwards compatible way, and would make the post-processing feature available for the batch mode.

Raw Data->Graph Collections Proposal

In batch mode, the data passed into the post-processing block is the "raw" data, unlike the data returned from the batch operation's execute method which are GraphCollections.

I'd like to propose to change this post-processing mechanism to pass in the GraphCollections, instead of raw data. It's less of a post-processing then and more of a data consumption mechanism, but given that the high-level API as written doesn't expose this feature anyway, it's probably not too bad of a breaking change.

@wolframarnold wolframarnold added a commit to wolframarnold/koala that referenced this issue Jul 19, 2012
@wolframarnold wolframarnold Instead of passing raw data to the post-processing block for batched …
…API calls, pass in the processed response (GraphCollections), so that we can use blocks to receive the data back. See Issue #237
80b3545
@arsduo
Owner

Hey Wolfram,

This looks good -- want to submit this as a pull request? I'd be happy to merge this in. This is, technically, a breaking change, but it's also a bug that the Batch API passed raw data when the main Graph API provides GraphCollection.evaluate'd objects.

Your API proposal sounds good -- it'd be backwards compatible, and particularly useful for the Batch API. Would you be interested in implementing that?

Alex

@wolframarnold
@wolframarnold wolframarnold added a commit to wolframarnold/koala that referenced this issue Jul 30, 2012
@wolframarnold wolframarnold [Issue #237] Implement proposed API change with tests:
Each Graph API method now takes an optional post-processing block. The result data is passed to the block for post-processing. The return value of the method is the result of the block, if provided.
This also now exposes the previously existing but hidden mechanism to pass a block to batched Graph API calls. The post-processing block received GraphCollections, not raw data. Technically
an API-breaking change, but the block interface was never exposed, so it wouldn't affect users of the high-level API's.
Tests included and Readme updated with examples.
bb62a10
@wolframarnold wolframarnold added a commit to wolframarnold/koala that referenced this issue Jul 30, 2012
@wolframarnold wolframarnold [Issue #237] Implement proposed API change with tests:
Each Graph API method now takes an optional post-processing block. The result data is passed to the block for post-processing. The return value of the method is the result of the block, if provided.
This also now exposes the previously existing but hidden mechanism to pass a block to batched Graph API calls. The post-processing block now receives GraphCollection objectss, not raw data. Technically
an API-breaking change, but the block interface was never exposed, so it wouldn't affect users of the high-level API's.
Tests included and Readme updated with examples.
9383cfd
@arsduo
Owner

Thanks for the pull request! I'll review that shortly and get it merged in. I really like the idea.

@arsduo arsduo closed this Aug 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment