You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by kxepal <gi...@git.apache.org> on 2015/04/21 14:52:47 UTC

[GitHub] couchdb-couch pull request: rebased _bulk_get patch

Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/18#issuecomment-94781101
  
    Hi there, short update.
    
    I'd started porting this PR over 2.0. First note that in current state it doesn't works and breaks any document update, so don't try to merge it (:
    
    Second, while fixing it I found that initial format doesn't assumes any possible errors:
    
    ```
    $ echo '{"docs": [{"id": "foo"}, {"id": "bar"}]}' | http http://localhost:15986/test/_bulk_get  
    [
        [{"_id": "foo", "_rev": "1-967a00dff5e02add41819138abb3284d"}],
        [{"_id": "bar", "_rev": "1-6aaf7080aad9d1a9482e07c46581dac0"}]
    ]
    ```
    
    What the response should be if document id is missed or wrong or document not exists or some parameters are malformed? Oblivious solution is:
    
    ```
    $ echo '{"docs": [{"id": "foo"}, {"id": "bar"}]}' | http http://localhost:15986/test/_bulk_get  
    [
        [{"_id": "foo", "_rev": "1-967a00dff5e02add41819138abb3284d"}],
        [{"error": "not_found", "id": "bar", "reason": "not today"}]
    ]
    ```
    
    But such array of arrays makes me sad panda.
    
    I'd started to look on current implementations and found no agreement on what the response should looks like:
    
    - [Couchbase](http://developer.couchbase.com/mobile/develop/references/sync-gateway/rest-api/database/post-bulk-get/index.html) assumes the response to be always multipart and request payload should contain array of objects where "id" field is mandatory, "rev" and "atts_since"  are optional and no mention of the others. So, passing open_revs will not work for Couchbase or make it incompatible with.
    
    - [RCouch](https://github.com/rcouch/couchdb-couch-httpd/blob/master/src/couch_httpd_bulk_get.erl#L45) returns JSON response in the fashion `{"results": [{"id": "foo", "docs": [...]}, ...]}` what is completely different from proposed patch and pouchdb-bulk-get implementation.
    
    - Each of these are incompatible with pouchdb-bulk-get project which was proposed as "standard implementation"
    
    Ok, now I'm really confused now. If I make /_bulk_get compatible with PouchDB that will means that it will be not compatible with the Couchbase/RCouch and be very annoying for people who'll try use it outside of replication context (it's stupid to not).
    
    If I start fixing oblivious flaws of proposed implementation, then we'll have yet another incompatible with the others /_bulk_get implementation. Not cool.
    
    At this moment I want to flip the table and make this feature from scratch, starting first from the actual proposal about the API of final implementation, finding agreement on this in CouchDB team and call PouchDB and Couchbase folks to the table (once I put it back) for the further implementation as a part of replication API. And make this short and quickly since @dholth made all the work, current devil is in the details and shape of things.
    
    Otherwise we (CouchDB) will end with semi-compatible with *ouchDB world resource that's broken for everyday usage. I'm -1 on such turn of events.
    
    Certainly, I may be wrong at all of this, so any ideas are welcome.
    
    Nudge list: @nolanlawson @dholth @janl @benoitc 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---