You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by chewbranca <gi...@git.apache.org> on 2015/09/29 22:38:21 UTC

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

GitHub user chewbranca opened a pull request:

    https://github.com/apache/couchdb-couch/pull/107

    Add revs limit for docs passed to filter functions

    The existing revs_limit logic only accounts for depth of individual
    branches, and ignores the case where you have a lot of branches, for
    instance when a doc is heavily conflicted. This patch truncates the list
    of revs when passing docs to the JS filter functions, because given
    enough revs this can cause the couchjs processes to explode.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/apache/couchdb-couch add-filter-revs-limit

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb-couch/pull/107.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #107
    
----
commit bb82dd6ae52d2d5b12ca6d16941c5f7fa2ea10ae
Author: Russell Branca <ch...@apache.org>
Date:   2015-09-29T20:32:02Z

    Add revs limit for docs passed to filter functions
    
    The existing revs_limit logic only accounts for depth of individual
    branches, and ignores the case where you have a lot of branches, for
    instance when a doc is heavily conflicted. This patch truncates the list
    of revs when passing docs to the JS filter functions, because given
    enough revs this can cause the couchjs processes to explode.

----


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r40949063
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -296,7 +296,8 @@ filter_docs(Req, Db, DDoc, FName, Docs) ->
         #httpd{} = HttpReq ->
             couch_httpd_external:json_req_obj(HttpReq, Db)
         end,
    -    JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    +    Limit = config:get_integer("couchdb", "filter_revs_limit", 20),
    --- End diff --
    
    I went with `couchdb` because `revs_limit` is part of the `db_header` record, but as this is for options going through the query server, that switch to `query_server_config` seems reasonable to me.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-144364357
  
    hrm, when filtering is there any point passing the _rev or rev history at all? (cc @davisp @kocolosk).


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r40932604
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -296,7 +296,8 @@ filter_docs(Req, Db, DDoc, FName, Docs) ->
         #httpd{} = HttpReq ->
             couch_httpd_external:json_req_obj(HttpReq, Db)
         end,
    -    JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    +    Limit = config:get_integer("couchdb", "filter_revs_limit", 20),
    --- End diff --
    
    May be use 
    ```
    Limit = config:get_integer("query_servers", "revs_limit", 20),
    ```
    
    ? Then we can reuse the same option for other design functions which receives _revisions list now.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-146953322
  
    which comment?


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-144771791
  
    @rnewson @kxepal When I first saw this my first thought was to remove _revisions entirely assuming that it was basically an accident to have included it in the first place. But given that its there and its theoretically possible someone might rely on it I instead suggested the truncation approach since revs_limit exists so anyone relying on it can't correctly rely on a specific length of the _revisions list.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r41792348
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -280,12 +287,23 @@ validate_doc_update(DDoc, EditDoc, DiskDoc, Ctx, SecObj) ->
                 throw({unknown_error, Message})
         end.
     
    -json_doc(nil) -> null;
    +json_doc_options() ->
    +    json_doc_options([]).
    +
    +json_doc_options(Options) ->
    +    Limit = config:get_integer("query_server_config", "revs_limit", 20),
    +    [{revs, Limit} | Options].
    +
     json_doc(Doc) ->
    -    couch_doc:to_json_obj(Doc, [revs]).
    +    json_doc(Doc, json_doc_options()).
    --- End diff --
    
    It was good idea to keep `json_doc(nil) -> null;` to not make config lookup when we don't actually need in it result.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-144772687
  
    hrm, fair enough, we can't be sure no one is depending on it. 
    
    +1


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r40952845
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -296,7 +296,8 @@ filter_docs(Req, Db, DDoc, FName, Docs) ->
         #httpd{} = HttpReq ->
             couch_httpd_external:json_req_obj(HttpReq, Db)
         end,
    -    JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    +    Limit = config:get_integer("couchdb", "filter_revs_limit", 20),
    --- End diff --
    
    Fixed in 1628a3a.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca closed the pull request at:

    https://github.com/apache/couchdb-couch/pull/107


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r40957982
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -296,7 +296,8 @@ filter_docs(Req, Db, DDoc, FName, Docs) ->
         #httpd{} = HttpReq ->
             couch_httpd_external:json_req_obj(HttpReq, Db)
         end,
    -    JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    +    Limit = config:get_integer("query_server_config", "filter_revs_limit", 20),
    --- End diff --
    
    @kxepal I looked at all the places that call `couch_query_server:json_doc/1` and I think you're right, that would be a good safe option to making the `revs_limit` logic work here for show, filter, and update functions. Any objections @davisp @rnewson?


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r41804497
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -280,12 +287,23 @@ validate_doc_update(DDoc, EditDoc, DiskDoc, Ctx, SecObj) ->
                 throw({unknown_error, Message})
         end.
     
    -json_doc(nil) -> null;
    +json_doc_options() ->
    +    json_doc_options([]).
    +
    +json_doc_options(Options) ->
    +    Limit = config:get_integer("query_server_config", "revs_limit", 20),
    +    [{revs, Limit} | Options].
    +
     json_doc(Doc) ->
    -    couch_doc:to_json_obj(Doc, [revs]).
    +    json_doc(Doc, json_doc_options()).
    --- End diff --
    
    I disagree, I took it out intentionally. I'm not a fan of duplicating that logic; the one arity `json_doc` function now exists to add default options, and should do nothing else. The two arity version handles all the actual logic. I did consider the impact of adding the extra config lookup before short circuiting on `nil`, but I don't think we should be worrying about the speed of config lookups from ETS in general, especially at the expense of clarity in the code.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-146908752
  
    @rnewson comment wasn't addressed. 


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r40967252
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -296,7 +296,8 @@ filter_docs(Req, Db, DDoc, FName, Docs) ->
         #httpd{} = HttpReq ->
             couch_httpd_external:json_req_obj(HttpReq, Db)
         end,
    -    JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    +    Limit = config:get_integer("query_server_config", "filter_revs_limit", 20),
    --- End diff --
    
    Nope.
    
    On Thu, Oct 1, 2015 at 2:50 PM, Russell Branca <no...@github.com>
    wrote:
    
    > In src/couch_query_servers.erl
    > <https://github.com/apache/couchdb-couch/pull/107#discussion_r40957982>:
    >
    > > @@ -296,7 +296,8 @@ filter_docs(Req, Db, DDoc, FName, Docs) ->
    > >      #httpd{} = HttpReq ->
    > >          couch_httpd_external:json_req_obj(HttpReq, Db)
    > >      end,
    > > -    JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    > > +    Limit = config:get_integer("query_server_config", "filter_revs_limit", 20),
    >
    > @kxepal <https://github.com/kxepal> I looked at all the places that call
    > couch_query_server:json_doc/1 and I think you're right, that would be a
    > good safe option to making the revs_limit logic work here for show,
    > filter, and update functions. Any objections @davisp
    > <https://github.com/davisp> @rnewson <https://github.com/rnewson>?
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/couchdb-couch/pull/107/files#r40957982>.
    >



---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-147537263
  
    Merged with https://github.com/apache/couchdb-couch/commit/eb60e41ad0afdad1281455e19cf1bf02419864d4 .


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-147538545
  
    @chewbranca how come this isn't marked 'merged' like usual?


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-147537257
  
    Merged in master.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-144365939
  
    @chewbranca Does this also affects on other design functions? IIRC shows also receives documents with the whole _revisions list by default now. Not sure about update ones, never was curious about.
    
    Another question is why filter function receives list of _revisions now? What the use case of this and may be it's better to not send it at all?


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-144772426
  
    @davisp I'm not sure about someone to relay on it. What's practical use case for them in filter function? String all docs which has more than N revisions in history? Sounds strange.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-147539992
  
    @rnewson I forgot to push back out the branch after I rebased my changes on top of master, so the sha's were different in master than here.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-144774463
  
    @davisp well, good point. Let's land this and run some poll later to deprecate and remove this. 
    
    However, we also need more general solution since here we improved filtered changes feed performance a lot, but still I can blow your query server with show and update function using the same vector. Having `filter_revs_limit`, `shows_revs_limit`, `update_revs_limit` config options is a bit weird.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r41792760
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -280,12 +287,23 @@ validate_doc_update(DDoc, EditDoc, DiskDoc, Ctx, SecObj) ->
                 throw({unknown_error, Message})
         end.
     
    -json_doc(nil) -> null;
    +json_doc_options() ->
    +    json_doc_options([]).
    +
    +json_doc_options(Options) ->
    +    Limit = config:get_integer("query_server_config", "revs_limit", 20),
    +    [{revs, Limit} | Options].
    +
     json_doc(Doc) ->
    -    couch_doc:to_json_obj(Doc, [revs]).
    +    json_doc(Doc, json_doc_options()).
    +
    +json_doc(nil, _) ->
    +    null;
    +json_doc(Doc, Options) ->
    +    couch_doc:to_json_obj(Doc, Options).
     
     filter_view(DDoc, VName, Docs) ->
    -    JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    +    JsonDocs = [json_doc(Doc) || Doc <- Docs],
    --- End diff --
    
    Here is better explicitly pass options because otherwise you'll query config for N times as much as Docs you have.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-147540226
  
    @rnewson Commit hashes in PR and on master are different. And merge commit doesn't contains magic works "This closes #107". One of these conditions need to be fixed to help ASFBot get to know which PR need to close/merge.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-146907855
  
    +1. @chewbranca merge pls 


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r41805298
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -280,12 +287,23 @@ validate_doc_update(DDoc, EditDoc, DiskDoc, Ctx, SecObj) ->
                 throw({unknown_error, Message})
         end.
     
    -json_doc(nil) -> null;
    +json_doc_options() ->
    +    json_doc_options([]).
    +
    +json_doc_options(Options) ->
    +    Limit = config:get_integer("query_server_config", "revs_limit", 20),
    +    [{revs, Limit} | Options].
    +
     json_doc(Doc) ->
    -    couch_doc:to_json_obj(Doc, [revs]).
    +    json_doc(Doc, json_doc_options()).
    --- End diff --
    
    Ok, you have a point. Thanks!


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r41804132
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -280,12 +287,23 @@ validate_doc_update(DDoc, EditDoc, DiskDoc, Ctx, SecObj) ->
                 throw({unknown_error, Message})
         end.
     
    -json_doc(nil) -> null;
    +json_doc_options() ->
    +    json_doc_options([]).
    +
    +json_doc_options(Options) ->
    +    Limit = config:get_integer("query_server_config", "revs_limit", 20),
    +    [{revs, Limit} | Options].
    +
     json_doc(Doc) ->
    -    couch_doc:to_json_obj(Doc, [revs]).
    +    json_doc(Doc, json_doc_options()).
    +
    +json_doc(nil, _) ->
    +    null;
    +json_doc(Doc, Options) ->
    +    couch_doc:to_json_obj(Doc, Options).
     
     filter_view(DDoc, VName, Docs) ->
    -    JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    +    JsonDocs = [json_doc(Doc) || Doc <- Docs],
    --- End diff --
    
    Sure, fixed this and the other place doing the same thing in 9c5b8b7.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-144773464
  
    @kxepal I haven't got a clue why someone might use it. But it exists so I'm guessing someone somewhere would be angry if we removed it.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-147531585
  
    after appropriate squashing


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch/pull/107#discussion_r40954334
  
    --- Diff: src/couch_query_servers.erl ---
    @@ -296,7 +296,8 @@ filter_docs(Req, Db, DDoc, FName, Docs) ->
         #httpd{} = HttpReq ->
             couch_httpd_external:json_req_obj(HttpReq, Db)
         end,
    -    JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    +    Limit = config:get_integer("query_server_config", "filter_revs_limit", 20),
    --- End diff --
    
    You still think it only applicable for filter functions? How about this:
    ```
    --- a/src/couch_query_servers.erl
    +++ b/src/couch_query_servers.erl
    @@ -282,7 +282,14 @@ validate_doc_update(DDoc, EditDoc, DiskDoc, Ctx, SecObj) ->
     
     json_doc(nil) -> null;
     json_doc(Doc) ->
    -    couch_doc:to_json_obj(Doc, [revs]).
    +    json_doc(Doc, json_doc_options()).
    +json_doc(Doc, Options) ->
    +    couch_doc:to_json_obj(Doc, Options).
    +
    +json_doc_options() ->
    +    Limit = config:get_integer("query_server_config", "revs_limit", 20),
    +    Options = [{revs, Limit}],
    +    Options.
     
     filter_view(DDoc, VName, Docs) ->
         JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    @@ -296,7 +303,8 @@ filter_docs(Req, Db, DDoc, FName, Docs) ->
         #httpd{} = HttpReq ->
             couch_httpd_external:json_req_obj(HttpReq, Db)
         end,
    -    JsonDocs = [couch_doc:to_json_obj(Doc, [revs]) || Doc <- Docs],
    +    Options = json_doc_options(),
    +    JsonDocs = [json_doc(Doc, Options) || Doc <- Docs],
         [true, Passes] = ddoc_prompt(DDoc, [<<"filters">>, FName],
             [JsonDocs, JsonReq]),
         {ok, Passes}.
    ```
    
    Then this option will get shared for [shows](https://github.com/apache/couchdb-couch-mrview/blob/master/src/couch_mrview_show.erl#L78) and [updates](https://github.com/apache/couchdb-couch-mrview/blob/master/src/couch_mrview_show.erl#L123) functions.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-147540367
  
    @chewbranca @kxepal it's certainly possible to do this 'right', can we be more careful for future PR's please.


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-147531400
  
    +1


---
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.
---

[GitHub] couchdb-couch pull request: Add revs limit for docs passed to filt...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch/pull/107#issuecomment-147541227
  
    @rnewsown Sure. Actually just need to always push changes to your feature branch before do the actual merge. If we'll ask for / do rebase against current master before all of that - would be awesome and this help us to avoid useless merge commits for single commit changes.


---
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.
---