couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kxepal <...@git.apache.org>
Subject [GitHub] couchdb-couch pull request: rebased _bulk_get patch
Date Tue, 21 Apr 2015 12:52:47 GMT
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.
---

Mime
View raw message