You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2020/06/19 17:44:45 UTC
[GitHub] [couchdb] bessbd opened a new pull request #2958: Allow drilldown for search to always be specified as list of lists
bessbd opened a new pull request #2958:
URL: https://github.com/apache/couchdb/pull/2958
<!-- Thank you for your contribution!
Please file this form by replacing the Markdown comments
with your text. If a section needs no action - remove it.
Also remember, that CouchDB uses the Review-Then-Commit (RTC) model
of code collaboration. Positive feedback is represented +1 from committers
and negative is a -1. The -1 also means veto, and needs to be addressed
to proceed. Once there are no objections, the PR can be merged by a
CouchDB committer.
See: http://couchdb.apache.org/bylaws.html#decisions for more info. -->
## Overview
<!-- Please give a short brief for the pull request,
what problem it solves or how it makes things better. -->
To use multiple `drilldown` parameters users had to define
`drilldown` multiple times to be able supply them.
This caused interoperability issues as most languages require
defining query parameters and request bodies as associative
arrays, maps or dictionaries where the keys are unique.
This change enables defining `drilldown` as a list of lists so
that other languages can define multiple drilldown keys and values.
## Testing recommendations
<!-- Describe how we can test your changes.
Does it provides any behaviour that the end users
could notice? -->
Run the tests included
## Related Issues or Pull Requests
<!-- If your changes affects multiple components in different
repositories please put links to those issues or pull requests here. -->
## Checklist
- [X] Code is written and works correctly
- [X] Changes are covered by tests
- [X] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation - working on it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] bessbd commented on pull request #2958: Allow drilldown for search to always be specified as list of lists
Posted by GitBox <gi...@apache.org>.
bessbd commented on pull request #2958:
URL: https://github.com/apache/couchdb/pull/2958#issuecomment-647415218
Thank you for the review, @garrensmith ! I've performed all the changes you've requested in f997160515c2c7d2ee4fdd31d591a08745911565
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] garrensmith commented on a change in pull request #2958: Allow drilldown for search to always be specified as list of lists
Posted by GitBox <gi...@apache.org>.
garrensmith commented on a change in pull request #2958:
URL: https://github.com/apache/couchdb/pull/2958#discussion_r443431868
##########
File path: src/dreyfus/src/dreyfus_httpd.erl
##########
@@ -31,11 +31,14 @@ handle_search_req(#httpd{method=Method, path_parts=[_, _, _, _, IndexName]}=Req
when Method == 'GET'; Method == 'POST' ->
DbName = couch_db:name(Db),
Start = os:timestamp(),
- QueryArgs = #index_query_args{
+ RawQueryArgs = #index_query_args{
include_docs = IncludeDocs,
grouping = Grouping
} = parse_index_params(Req, Db),
- validate_search_restrictions(Db, DDoc, QueryArgs),
+ validate_search_restrictions(Db, DDoc, RawQueryArgs),
+ QueryArgs = RawQueryArgs#index_query_args{
+ drilldown= unwrap_list_of_list_of_lists(RawQueryArgs#index_query_args.drilldown)
Review comment:
You are missing a space after `drilldown`
##########
File path: src/dreyfus/src/dreyfus_httpd.erl
##########
@@ -31,11 +31,14 @@ handle_search_req(#httpd{method=Method, path_parts=[_, _, _, _, IndexName]}=Req
when Method == 'GET'; Method == 'POST' ->
DbName = couch_db:name(Db),
Start = os:timestamp(),
- QueryArgs = #index_query_args{
+ RawQueryArgs = #index_query_args{
Review comment:
Rather make this `QueryArgs0`
And then later make it `QueryArgs1`
##########
File path: src/dreyfus/src/dreyfus_httpd.erl
##########
@@ -209,6 +212,16 @@ parse_index_params(IndexParams, Db) ->
validate_index_query(K, V, Args2)
end, Args, IndexParams).
+unwrap_list_of_list_of_lists([Inner]=Outer) when is_list(Inner) ->
Review comment:
Can you rename this to what its actually doing and leave a comment explaining the list of lists of lists thing with drilldowns.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson merged pull request #2958: Allow drilldown for search to always be specified as list of lists
Posted by GitBox <gi...@apache.org>.
rnewson merged pull request #2958:
URL: https://github.com/apache/couchdb/pull/2958
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] garrensmith commented on pull request #2958: Allow drilldown for search to always be specified as list of lists
Posted by GitBox <gi...@apache.org>.
garrensmith commented on pull request #2958:
URL: https://github.com/apache/couchdb/pull/2958#issuecomment-647574982
Nice that fix looks way better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] bessbd commented on pull request #2958: Allow drilldown for search to always be specified as list of lists
Posted by GitBox <gi...@apache.org>.
bessbd commented on pull request #2958:
URL: https://github.com/apache/couchdb/pull/2958#issuecomment-647995430
Thank you for the reviews, @garrensmith and @rnewson ! Also, thank you for the merge, @rnewson
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org