[GitHub] nickva commented on a change in pull request #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
nickva commented on a change in pull request #1195: Add support for bulk get
with Accept:"multipart/mixed" or "multipart/related"
URL: https://github.com/apache/couchdb/pull/1195#discussion_r232427519
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -481,18 +482,62 @@ db_req(#httpd{method='POST', path_parts=[_,
<<"_bulk_get">>]}=Req, Db) ->
} = bulk_get_parse_doc_query(Req),
Options = [{user_ctx, Req#httpd.user_ctx} | Options0],
-{ok, Resp} = start_json_response(Req, 200),
-send_chunk(Resp, <<"{\"results\": [">>),
-
-lists:foldl(fun(Doc, Sep) ->
-{DocId, Results, Options1} = bulk_get_open_doc_revs(Db, Doc,
-Options),
-bulk_get_send_docs_json(Resp, DocId, Results, Options1, Sep),
-<<",">>
-end, <<"">>, Docs),
-
-send_chunk(Resp, <<"]}">>),
-end_json_response(Resp)
+% Decide whether to use multipart response or application/json
+AcceptMixedMp = MochiReq:accepts_content_type("multipart/mixed"),
+AcceptRelatedMp =
MochiReq:accepts_content_type("multipart/related"),
+AcceptMp = AcceptMixedMp orelse AcceptRelatedMp,
Review comment:
Sorry one more thing I realized. I think this logic would make multipart
response become the default if there is no accept header or it accepts `*/*`.
In general the semantics would be correct, but given that this is a new
behavior, it would be better if in such cases we returned json, and only if
json is not accepted but multipart is then we send multipart.
So I think we'd want something like:
```
AcceptJson = MochiReq:accepts_content_type("application/json"),
...
SendMp = not AcceptJson andalso (AcceptMixedMp orelse AcceptRelatedMp)
case SendMp of ...
```
So we are saying we only send mp if json wasn't requested (either because of
a */* accept header or explicitly) and one of the mp flavors was. Otherwise we
send json as before.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] nickva commented on a change in pull request #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
nickva commented on a change in pull request #1195: Add support for bulk get
with Accept:"multipart/mixed" or "multipart/related"
URL: https://github.com/apache/couchdb/pull/1195#discussion_r232427519
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -481,18 +482,62 @@ db_req(#httpd{method='POST', path_parts=[_,
<<"_bulk_get">>]}=Req, Db) ->
} = bulk_get_parse_doc_query(Req),
Options = [{user_ctx, Req#httpd.user_ctx} | Options0],
-{ok, Resp} = start_json_response(Req, 200),
-send_chunk(Resp, <<"{\"results\": [">>),
-
-lists:foldl(fun(Doc, Sep) ->
-{DocId, Results, Options1} = bulk_get_open_doc_revs(Db, Doc,
-Options),
-bulk_get_send_docs_json(Resp, DocId, Results, Options1, Sep),
-<<",">>
-end, <<"">>, Docs),
-
-send_chunk(Resp, <<"]}">>),
-end_json_response(Resp)
+% Decide whether to use multipart response or application/json
+AcceptMixedMp = MochiReq:accepts_content_type("multipart/mixed"),
+AcceptRelatedMp =
MochiReq:accepts_content_type("multipart/related"),
+AcceptMp = AcceptMixedMp orelse AcceptRelatedMp,
Review comment:
Sorry one more thing I realized. I think this logic would make multipart
response become the default if there is no accept header or it accepts '*/*'.
In general the semantics would be correct, but given that this is a new
behavior, it would be better if in such cases we returned json, and only if
json is not accepted but multipart is then we send multipart.
So I think we'd want something like:
```
AcceptJson = MochiReq:accepts_content_type("application/json"),
...
SendMp = not AcceptJson andalso (AcceptMixedMp orelse AcceptRelatedMp)
case SendMp of ...
```
So we are saying we only send mp if json wasn't requested (either because of
a */* accept header or explicitly) and one of the mp flavors was. Otherwise we
send json as before.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[Jenkins] FAILURE: CouchDB » master #391
Boo, we failed. https://builds.apache.org/job/CouchDB/job/master/391/
[GitHub] nickva closed pull request #1722: Retain replication stats between job runs
nickva closed pull request #1722: Retain replication stats between job runs URL: https://github.com/apache/couchdb/pull/1722 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jaydoane commented on a change in pull request #1722: Retain replication stats between job runs
jaydoane commented on a change in pull request #1722: Retain replication stats
between job runs
URL: https://github.com/apache/couchdb/pull/1722#discussion_r232361229
##
File path: src/couch_replicator/src/couch_replicator_scheduler_job.erl
##
@@ -159,16 +159,9 @@ do_init(#rep{options = Options, id = {BaseId, Ext},
user_ctx=UserCtx} = Rep) ->
{source, ?l2b(SourceName)},
{target, ?l2b(TargetName)},
{continuous, get_value(continuous, Options, false)},
-{revisions_checked, 0},
-{missing_revisions_found, 0},
-{docs_read, 0},
-{docs_written, 0},
-{changes_pending, get_pending_count(State)},
-{doc_write_failures, 0},
{source_seq, HighestSeq},
-{checkpointed_source_seq, CommittedSeq},
{checkpoint_interval, CheckpointInterval}
-]),
+] ++ rep_stats(State)),
Review comment:
It seems like this refactor introduced a new warning:
```
db/src/couchdb/src/couch_replicator/src/couch_replicator_scheduler_job.erl:121:
Warning: variable 'CommittedSeq' is unused
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wohali commented on issue #328: feat: update docs for new query server config
wohali commented on issue #328: feat: update docs for new query server config URL: https://github.com/apache/couchdb-documentation/pull/328#issuecomment-437497524 Think this needs updating for changes to SSL config still. We fully scrubbed out proxy stuff and os_daemons already. Don't think we need to document configuring the http global handlers, but if you want, go for it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[Jenkins] SUCCESS: CouchDB » master #390
Yay, we passed. https://builds.apache.org/job/CouchDB/job/master/390/
[GitHub] wohali commented on issue #1724: [Feature Proposal] Pull Request to add support for Erlang-only validate_doc_read functions
wohali commented on issue #1724: [Feature Proposal] Pull Request to add support for Erlang-only validate_doc_read functions URL: https://github.com/apache/couchdb/issues/1724#issuecomment-437473945 I'm -0.5 on the proposal. Concerns: 1) Overall throughput decrease if a database doesn't have any of these new functions, just from having to traverse your new code path on *every read*, 2) Lack of security and sandboxing in Erlang-based functions, which is why we ship with the Erlang query server disabled by default I know that point 2 above is why this feature will never land at Cloudant and would likely never land at anyone running a public/SaaS CouchDB offering. A potential alternative would be #1554 's approach. The only way to handling point 1 above is to actually have proper load testing & metrics showing the impact. This is something Cloudant devs have done with their major functionality changes in PRs over the past couple of years. Do you have any data points to share? The alternative to this was already proposed by the committer list and is in the roadmap is the per-document permissions: the `_access` proposal. See https://lists.apache.org/thread.html/6aa77dd8e5974a3a540758c6902ccb509ab5a2e4802ecf4fd724a5e4@%3Cdev.couchdb.apache.org%3E This approach already has a bunch of code behind it as well, and would vastly improve on the current db-per-user access model. With `_access` would we even need `validate_doc_read` functions? I don't think so. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] janl closed pull request #1723: feat: update couch-config to match #1602
janl closed pull request #1723: feat: update couch-config to match #1602 URL: https://github.com/apache/couchdb/pull/1723 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] Antonio-Maranhao opened a new issue #1156: Add Nightwatch tests to validated partitioned database functions
Antonio-Maranhao opened a new issue #1156: Add Nightwatch tests to validated partitioned database functions URL: https://github.com/apache/couchdb-fauxton/issues/1156 Currently there are no Nightwatch tests against partitioned databases because the feature is not yet available on the Docker image. Once a suitable Docker image is released, some GUI tests should be added to validate: - Navigation - Selected partition key is preserved - Updating/clearing a partition key updates the list of results - View/Edit/Create partitioned views - Run partitioned Mango queries - Create partitioned indexes (Mango page) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] Antonio-Maranhao opened a new pull request #1155: [partitioned dbs] Support partitioned mango queries and creating partitioned indexes
Antonio-Maranhao opened a new pull request #1155: [partitioned dbs] Support
partitioned mango queries and creating partitioned indexes
URL: https://github.com/apache/couchdb-fauxton/pull/1155
## Overview
Sixth in a series of PRs that will be submitted in support of the new
user-defined partitioned databases feature (apache/couchdb#1605).
Summary:
- Show results for partitioned mango queries
- Create partitioned indexes
## Testing recommendations
On a partitioned database:
- Go to **Run A Query with Mango**
- Verify the Partition Selector is displayed at the top
- Click on **Run Query**
- Verify all docs that match are returned
- Set a partition key
- Verify the results are updated with only those that match the key
- Click on **Explain**
- Verify the results include the selected partition under `"opts"`
- Click on **manage indexes**
- Verify the **partitioned** checkbox is displayed and it is checked
- Click on **Create Index**
- Verify the results include the new index, and the index code includes
`"design_opts": { "partitioned": true }`
- Uncheck the **partitioned** checkbox and click on **Create Index**
- Verify the results include the new index, and the index code includes
`"design_opts": { "partitioned": false }`
- Add `partitioned: true` to the index code in the editor, make sure the
checkbox is unchecked, then click on **Create Index**
- Verify the results include the new index, and the index code includes
`"design_opts": { "partitioned": true }` - which means the value in the editor
takes precedence over the checkbox.
## Related Pull Requests
apache/couchdb#1605
## Checklist
- [x] Code is written and works correctly;
- [x] Changes are covered by tests;
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437447396 @nickva Thanks! Indeed it was some intermittent failure. All is good now. Let me know if I need to address something else before this can be merged into `master`. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437447396 @nickva Thanks! Indeed it was some sporadic fail. All is good now. Let me know if I need to address something else before this can be merged into `master`. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] nickva commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
nickva commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437435048 Ah it's probably flaky, I'll try to restart it This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437421418 @nickva I checked Travis logs and seemingly only one job `Job #6237.5` for OTP 17.5 failed for some odd reason. All other ones are successful as well as `chttpd` for the failed one. I've skimmed through the logs of this job and I do believe this error has nothing to do with my functionality. Perhaps some configs are wrong: ``` Failed: 0. Skipped: 0. Passed: 1004. One or more tests were cancelled. ERROR: One or more eunit tests failed. ERROR: eunit failed while processing /home/travis/build/apache/couchdb/src/couch: rebar_abort make[1]: *** [eunit] Error 1 make[1]: Leaving directory `/home/travis/build/apache/couchdb' make: *** [check] Error 2 The command "make check" exited with 2. ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437421418 @nickva I checked Travis logs and seemingly only one job `Job #6237.5` for OTP 17.5 failed for some odd reason. All other ones are successful. I've skimmed through the logs of this job and I do believe this error has nothing to do with my functionality. Perhaps some configs are wrong: ``` Failed: 0. Skipped: 0. Passed: 1004. One or more tests were cancelled. ERROR: One or more eunit tests failed. ERROR: eunit failed while processing /home/travis/build/apache/couchdb/src/couch: rebar_abort make[1]: *** [eunit] Error 1 make[1]: Leaving directory `/home/travis/build/apache/couchdb' make: *** [check] Error 2 The command "make check" exited with 2. ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437421418 @nickva I checked Travis logs and seemingly only one job `Job #6237.5` for OTP 17.5 failed for some odd reason. All other ones are successful. I've skimmed through the logs of this job and I do believe this error has nothing to do with my functionality. Perhaps some configs are wrong: ``` ERROR: One or more eunit tests failed. ERROR: eunit failed while processing /home/travis/build/apache/couchdb/src/couch: rebar_abort make[1]: *** [eunit] Error 1 make[1]: Leaving directory `/home/travis/build/apache/couchdb' make: *** [check] Error 2 The command "make check" exited with 2. ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437421418 @nickva I checked Travis logs and seemingly only one job `Job #6237.5` for OTP 17.5 failed for some odd reason. All other ones are successful. I've checked the logs of this job and I do believe this error has nothing to do with my functionality. Perhaps some configs are wrong: ``` ERROR: One or more eunit tests failed. ERROR: eunit failed while processing /home/travis/build/apache/couchdb/src/couch: rebar_abort make[1]: *** [eunit] Error 1 make[1]: Leaving directory `/home/travis/build/apache/couchdb' make: *** [check] Error 2 The command "make check" exited with 2. ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437421418 @nickva I checked Travis logs and seemingly only one job `Job #6237.5` for OTP 17.5 failed for some odd reason. All other ones are successful. I've checked the logs and I do believe this error has nothing to do with my functionality. Perhaps some configs are wrong: ``` ERROR: One or more eunit tests failed. ERROR: eunit failed while processing /home/travis/build/apache/couchdb/src/couch: rebar_abort make[1]: *** [eunit] Error 1 make[1]: Leaving directory `/home/travis/build/apache/couchdb' make: *** [check] Error 2 The command "make check" exited with 2. ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zacharylawson commented on issue #323: Update purging.rst
zacharylawson commented on issue #323: Update purging.rst URL: https://github.com/apache/couchdb-documentation/pull/323#issuecomment-437414064 @jiangphcn my pleasure, updated the branch. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437410621 Valid point. `meck:unload()` has been incorporated. As for failed test this failure is due to the fact `Options` list which is passed to the `fabric_doc_open_revs:go()` has to contain `user_ctx`. Fixed as well. Reran `chttpd` tests locally: `All 232 tests passed.` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437410621 Valid point. `meck:unload()` has been incorporated. As for failing test this failure is due to the fact `Options` list which is passed to the `fabric_doc_open_revs:go()` has to contain `user_ctx`. Fixed as well. Reran `chttpd` tests locally: `All 232 tests passed.` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov commented on a change in pull request #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov commented on a change in pull request #1195: Add support for
bulk get with Accept:"multipart/mixed" or "multipart/related"
URL: https://github.com/apache/couchdb/pull/1195#discussion_r232308403
##
File path: src/chttpd/test/chttpd_db_bulk_get_multipart_test.erl
##
@@ -0,0 +1,309 @@
+%% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+%% use this file except in compliance with the License. You may obtain a copy
of
+%% the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+%% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+%% License for the specific language governing permissions and limitations
under
+%% the License.
+
+-module(chttpd_db_bulk_get_multipart_test).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+
+-define(TIMEOUT, 3000).
+
+
+setup() ->
+mock(config),
+mock(chttpd),
+mock(couch_epi),
+mock(couch_httpd),
+mock(couch_stats),
+mock(fabric),
+mock(mochireq),
+Pid = spawn_accumulator(),
+Pid.
+
+
+teardown(Pid) ->
+ok = stop_accumulator(Pid),
+meck:unload(config),
Review comment:
This is my blunder. I overlooked that `meck` has this convenient function.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] nickva edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
nickva edited a comment on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437400416 Looks much better. Nice work cleaning it up. I see eunit test failures locally and in Travis. Not sure exactly why, maybe because of the name change of the function or maybe something on master changed? Also see a minor comment about simplifying meck unloading. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] nickva commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
nickva commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437400416 Look better. Nice work cleaning it up. I see eunit test failures locally and in Travis. Not sure exactly why, maybe because of the name change of the function or maybe something on master changed? Also see a minor comment about simplifying meck unloading. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] nickva commented on a change in pull request #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
nickva commented on a change in pull request #1195: Add support for bulk get
with Accept:"multipart/mixed" or "multipart/related"
URL: https://github.com/apache/couchdb/pull/1195#discussion_r232297634
##
File path: src/chttpd/test/chttpd_db_bulk_get_multipart_test.erl
##
@@ -0,0 +1,309 @@
+%% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+%% use this file except in compliance with the License. You may obtain a copy
of
+%% the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+%% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+%% License for the specific language governing permissions and limitations
under
+%% the License.
+
+-module(chttpd_db_bulk_get_multipart_test).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+
+-define(TIMEOUT, 3000).
+
+
+setup() ->
+mock(config),
+mock(chttpd),
+mock(couch_epi),
+mock(couch_httpd),
+mock(couch_stats),
+mock(fabric),
+mock(mochireq),
+Pid = spawn_accumulator(),
+Pid.
+
+
+teardown(Pid) ->
+ok = stop_accumulator(Pid),
+meck:unload(config),
Review comment:
I think you can just do one meck:unload() and it will unload all mocked
modules.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] smithsz opened a new pull request #130: Only return Promise from `headDoc` when no callback is specified.
smithsz opened a new pull request #130: Only return Promise from `headDoc` when no callback is specified. URL: https://github.com/apache/couchdb-nano/pull/130 ## Overview Only return a `Promise` from `headDoc` when no user callback is specified. ## Testing recommendations Added additional unit tests. ## Checklist - [x] Code is written and works correctly; - [x] Changes are covered by tests; - [x] Documentation reflects the changes; This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[Jenkins] SUCCESS: CouchDB » master #389
Yay, we passed. https://builds.apache.org/job/CouchDB/job/master/389/
[GitHub] janl opened a new pull request #1723: feat: update couch-config to match #1602
janl opened a new pull request #1723: feat: update couch-config to match #1602 URL: https://github.com/apache/couchdb/pull/1723 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] couchdb-config pull request #20: fix tests for #1602
Github user asfgit closed the pull request at: https://github.com/apache/couchdb-config/pull/20 ---
[GitHub] janl closed pull request #1602: Remove runtime configurability of core system components
janl closed pull request #1602: Remove runtime configurability of core system components URL: https://github.com/apache/couchdb/pull/1602 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov commented on issue #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related" URL: https://github.com/apache/couchdb/pull/1195#issuecomment-437357434 Hello @nickva Once again thank you for a thorough review and your comments. I've addressed all the requested changes as well as fixed formatting style for the whole multipart case clause. All the commits squashed into one, and branch is rebased onto `master` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] tudordumitriu commented on issue #1712: Validate Document Update oldDoc in ddoc is null after doc PUT for n=1 server
tudordumitriu commented on issue #1712: Validate Document Update oldDoc in ddoc is null after doc PUT for n=1 server URL: https://github.com/apache/couchdb/issues/1712#issuecomment-437343263 Here it is a clean log after enabling debug This problem occurs randomly and by chance I just got over it again on a different doc [couch.log](https://github.com/apache/couchdb/files/2565771/couch.log) Thank you This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] janl edited a comment on issue #1602: Remove runtime configurability of core system components
janl edited a comment on issue #1602: Remove runtime configurability of core system components URL: https://github.com/apache/couchdb/pull/1602#issuecomment-421311043 Depends on: https://github.com/apache/couchdb-config/pull/20 Docs PR: https://github.com/apache/couchdb-documentation/pull/328 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] janl commented on issue #1602: Remove runtime configurability of core system components
janl commented on issue #1602: Remove runtime configurability of core system components URL: https://github.com/apache/couchdb/pull/1602#issuecomment-437319020 Pending any test issues and/or review notes, this is good to go now. Thanks everyone for the input! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] janl commented on issue #1602: Remove runtime configurability of core system components
janl commented on issue #1602: Remove runtime configurability of core system components URL: https://github.com/apache/couchdb/pull/1602#issuecomment-437317772 > @janl How is httpsd enabled in this new world? Previously that would have been: @wohali good one, I’ve made this analogous to the native query server: 1. if the legacy config is found, it is treated as option 2. set to `true` 2. now users can do `[ssl] enable = BOOL` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] AlexanderKaraberov commented on a change in pull request #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov commented on a change in pull request #1195: Add support for
bulk get with Accept:"multipart/mixed" or "multipart/related"
URL: https://github.com/apache/couchdb/pull/1195#discussion_r232206895
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -476,19 +477,59 @@ db_req(#httpd{method='POST', path_parts=[_,
<<"_bulk_get">>]}=Req, Db) ->
#doc_query_args{
options = Options
} = bulk_get_parse_doc_query(Req),
-
-{ok, Resp} = start_json_response(Req, 200),
-send_chunk(Resp, <<"{\"results\": [">>),
-
-lists:foldl(fun(Doc, Sep) ->
-{DocId, Results, Options1} = bulk_get_open_doc_revs(Db, Doc,
-Options),
-bulk_get_send_docs_json(Resp, DocId, Results, Options1, Sep),
-<<",">>
-end, <<"">>, Docs),
-
-send_chunk(Resp, <<"]}">>),
-end_json_response(Resp)
+% Decide whether to use multipart response or application/json
+AcceptMixedMp = MochiReq:accepts_content_type("multipart/mixed"),
+AcceptRelatedMp =
MochiReq:accepts_content_type("multipart/related"),
+AcceptMp = AcceptMixedMp orelse AcceptRelatedMp,
+case AcceptMp of
+ false ->
+ {ok, Resp} = start_json_response(Req, 200),
+ send_chunk(Resp, <<"{\"results\": [">>),
+ lists:foldl(fun(Doc, Sep) ->
+ {DocId, Results, Options1} = bulk_get_open_doc_revs(Db,
Doc,
+
Options),
+ bulk_get_send_docs_json(Resp, DocId, Results, Options1,
Sep),
+ <<",">>
+ end, <<"">>, Docs),
+ send_chunk(Resp, <<"]}">>),
+ end_json_response(Resp);
+ true ->
+ OuterBoundary = bulk_get_multipart_boundary(),
+ MpType = case AcceptMixedMp of
+ true ->
+ "multipart/mixed";
+ _ ->
+ "multipart/related"
+ end,
+ CType = {"Content-Type", MpType ++ "; boundary=\"" ++
+?b2l(OuterBoundary) ++ "\""},
+ {ok, Resp} = start_chunked_response(Req, 200, [CType]),
+ lists:foldl(fun(Doc, _Pre) ->
+ case bulk_get_open_doc_revs(Db, Doc, Options) of
+ {_, {ok, []}, _Options1} ->
+ ok;
+ {_, {ok, Results}, Options1} ->
+ send_docs_multipart(bulk_get, Req, Results,
Options1,
+ OuterBoundary, Resp);
+ {DocId, {error, {RevId, Error, Reason}}, _Options1}
->
+ Json = ?JSON_ENCODE({[{<<"id">>, DocId},
+{<<"rev">>, RevId},
+{<<"error">>, Error},
+{<<"reason">>, Reason}]}),
+ couch_httpd:send_chunk(Resp,
+ [<<"\r\n--", OuterBoundary/binary>>,
+ <<"\r\nContent-Type: application/json;
error=\"true\"\r\n\r\n">>,
+ Json])
+ end
+ end, <<"">>, Docs),
+ case Docs of
+ [] ->
+ ok;
+ _ ->
+ couch_httpd:send_chunk(Resp, <<"\r\n", "--",
OuterBoundary/binary, "--\r\n">>)
+ end,
+couch_httpd:last_chunk(Resp)
Review comment:
@nickva Actually it won't. This call is a part of multipart case clause.
Misapprehension in this case again was caused by a poor formatting style. I
addressed this and put a proper indentation to the whole clause.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] AlexanderKaraberov commented on a change in pull request #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov commented on a change in pull request #1195: Add support for
bulk get with Accept:"multipart/mixed" or "multipart/related"
URL: https://github.com/apache/couchdb/pull/1195#discussion_r232206895
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -476,19 +477,59 @@ db_req(#httpd{method='POST', path_parts=[_,
<<"_bulk_get">>]}=Req, Db) ->
#doc_query_args{
options = Options
} = bulk_get_parse_doc_query(Req),
-
-{ok, Resp} = start_json_response(Req, 200),
-send_chunk(Resp, <<"{\"results\": [">>),
-
-lists:foldl(fun(Doc, Sep) ->
-{DocId, Results, Options1} = bulk_get_open_doc_revs(Db, Doc,
-Options),
-bulk_get_send_docs_json(Resp, DocId, Results, Options1, Sep),
-<<",">>
-end, <<"">>, Docs),
-
-send_chunk(Resp, <<"]}">>),
-end_json_response(Resp)
+% Decide whether to use multipart response or application/json
+AcceptMixedMp = MochiReq:accepts_content_type("multipart/mixed"),
+AcceptRelatedMp =
MochiReq:accepts_content_type("multipart/related"),
+AcceptMp = AcceptMixedMp orelse AcceptRelatedMp,
+case AcceptMp of
+ false ->
+ {ok, Resp} = start_json_response(Req, 200),
+ send_chunk(Resp, <<"{\"results\": [">>),
+ lists:foldl(fun(Doc, Sep) ->
+ {DocId, Results, Options1} = bulk_get_open_doc_revs(Db,
Doc,
+
Options),
+ bulk_get_send_docs_json(Resp, DocId, Results, Options1,
Sep),
+ <<",">>
+ end, <<"">>, Docs),
+ send_chunk(Resp, <<"]}">>),
+ end_json_response(Resp);
+ true ->
+ OuterBoundary = bulk_get_multipart_boundary(),
+ MpType = case AcceptMixedMp of
+ true ->
+ "multipart/mixed";
+ _ ->
+ "multipart/related"
+ end,
+ CType = {"Content-Type", MpType ++ "; boundary=\"" ++
+?b2l(OuterBoundary) ++ "\""},
+ {ok, Resp} = start_chunked_response(Req, 200, [CType]),
+ lists:foldl(fun(Doc, _Pre) ->
+ case bulk_get_open_doc_revs(Db, Doc, Options) of
+ {_, {ok, []}, _Options1} ->
+ ok;
+ {_, {ok, Results}, Options1} ->
+ send_docs_multipart(bulk_get, Req, Results,
Options1,
+ OuterBoundary, Resp);
+ {DocId, {error, {RevId, Error, Reason}}, _Options1}
->
+ Json = ?JSON_ENCODE({[{<<"id">>, DocId},
+{<<"rev">>, RevId},
+{<<"error">>, Error},
+{<<"reason">>, Reason}]}),
+ couch_httpd:send_chunk(Resp,
+ [<<"\r\n--", OuterBoundary/binary>>,
+ <<"\r\nContent-Type: application/json;
error=\"true\"\r\n\r\n">>,
+ Json])
+ end
+ end, <<"">>, Docs),
+ case Docs of
+ [] ->
+ ok;
+ _ ->
+ couch_httpd:send_chunk(Resp, <<"\r\n", "--",
OuterBoundary/binary, "--\r\n">>)
+ end,
+couch_httpd:last_chunk(Resp)
Review comment:
@nickva Actually it won't. This call is a part of multipart case clause.
Misapprehension in this case again was caused by a poor formatting style. I've
addressed this and put a proper indentation to the whole clause.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] AlexanderKaraberov commented on a change in pull request #1195: Add support for bulk get with Accept:"multipart/mixed" or "multipart/related"
AlexanderKaraberov commented on a change in pull request #1195: Add support for
bulk get with Accept:"multipart/mixed" or "multipart/related"
URL: https://github.com/apache/couchdb/pull/1195#discussion_r232206895
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -476,19 +477,59 @@ db_req(#httpd{method='POST', path_parts=[_,
<<"_bulk_get">>]}=Req, Db) ->
#doc_query_args{
options = Options
} = bulk_get_parse_doc_query(Req),
-
-{ok, Resp} = start_json_response(Req, 200),
-send_chunk(Resp, <<"{\"results\": [">>),
-
-lists:foldl(fun(Doc, Sep) ->
-{DocId, Results, Options1} = bulk_get_open_doc_revs(Db, Doc,
-Options),
-bulk_get_send_docs_json(Resp, DocId, Results, Options1, Sep),
-<<",">>
-end, <<"">>, Docs),
-
-send_chunk(Resp, <<"]}">>),
-end_json_response(Resp)
+% Decide whether to use multipart response or application/json
+AcceptMixedMp = MochiReq:accepts_content_type("multipart/mixed"),
+AcceptRelatedMp =
MochiReq:accepts_content_type("multipart/related"),
+AcceptMp = AcceptMixedMp orelse AcceptRelatedMp,
+case AcceptMp of
+ false ->
+ {ok, Resp} = start_json_response(Req, 200),
+ send_chunk(Resp, <<"{\"results\": [">>),
+ lists:foldl(fun(Doc, Sep) ->
+ {DocId, Results, Options1} = bulk_get_open_doc_revs(Db,
Doc,
+
Options),
+ bulk_get_send_docs_json(Resp, DocId, Results, Options1,
Sep),
+ <<",">>
+ end, <<"">>, Docs),
+ send_chunk(Resp, <<"]}">>),
+ end_json_response(Resp);
+ true ->
+ OuterBoundary = bulk_get_multipart_boundary(),
+ MpType = case AcceptMixedMp of
+ true ->
+ "multipart/mixed";
+ _ ->
+ "multipart/related"
+ end,
+ CType = {"Content-Type", MpType ++ "; boundary=\"" ++
+?b2l(OuterBoundary) ++ "\""},
+ {ok, Resp} = start_chunked_response(Req, 200, [CType]),
+ lists:foldl(fun(Doc, _Pre) ->
+ case bulk_get_open_doc_revs(Db, Doc, Options) of
+ {_, {ok, []}, _Options1} ->
+ ok;
+ {_, {ok, Results}, Options1} ->
+ send_docs_multipart(bulk_get, Req, Results,
Options1,
+ OuterBoundary, Resp);
+ {DocId, {error, {RevId, Error, Reason}}, _Options1}
->
+ Json = ?JSON_ENCODE({[{<<"id">>, DocId},
+{<<"rev">>, RevId},
+{<<"error">>, Error},
+{<<"reason">>, Reason}]}),
+ couch_httpd:send_chunk(Resp,
+ [<<"\r\n--", OuterBoundary/binary>>,
+ <<"\r\nContent-Type: application/json;
error=\"true\"\r\n\r\n">>,
+ Json])
+ end
+ end, <<"">>, Docs),
+ case Docs of
+ [] ->
+ ok;
+ _ ->
+ couch_httpd:send_chunk(Resp, <<"\r\n", "--",
OuterBoundary/binary, "--\r\n">>)
+ end,
+couch_httpd:last_chunk(Resp)
Review comment:
@nickva Actually it won't. This call is a part of multipart case clause,
misapprehension in this case again was caused by a poor formatting style. I
addressed this and put a proper indentation to the whole clause.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] janl edited a comment on issue #1602: Remove runtime configurability of core system components
janl edited a comment on issue #1602: Remove runtime configurability of core system components URL: https://github.com/apache/couchdb/pull/1602#issuecomment-437304265 > also noting that changing couchjs stack size is an important ability. Since query servers can still be setup using env vars, the solution to this already exists: ``` export COUCHDB_QUERY_SERVER_JAVASCRIPT="/path/to/couchjs /path/to/main.js -S STACK SIZE" couchdb ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] janl commented on a change in pull request #1602: Remove runtime configurability of core system components
janl commented on a change in pull request #1602: Remove runtime
configurability of core system components
URL: https://github.com/apache/couchdb/pull/1602#discussion_r232193171
##
File path: src/couch/src/couch_secondary_sup.erl
##
@@ -26,6 +26,16 @@ init([]) ->
worker,
dynamic}
],
+Daemons = [
+{"index_server", "{couch_index_server, start_link, []}"},
Review comment:
Latest push removes the redundant string parsing. I’d accept commits that
turn the list comprehension into a helper, or one that writes it all out.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] janl commented on issue #1602: Remove runtime configurability of core system components
janl commented on issue #1602: Remove runtime configurability of core system components URL: https://github.com/apache/couchdb/pull/1602#issuecomment-437304265 > also noting that changing couchjs stack size is an important ability. Since query servers can still be setup using env vars, the solution to this already exists: `export COUCHDB_QUERY_SERVER_JAVASCRIPT="/path/to/couchjs /path/to/main.js -S STACK SIZE" couchdb` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] janl commented on issue #1602: Remove runtime configurability of core system components
janl commented on issue #1602: Remove runtime configurability of core system components URL: https://github.com/apache/couchdb/pull/1602#issuecomment-437303960 > I'm +1 on removing runtime configurability but would prefer that the configuration is expressed in the .app files of the respective erlang applications. This will allow package maintainers (etc) to more easily tweak at build time if necessary without introducing the runtime malleability that concerns us. @rnewson latest push defines all http handlers in `couch.app`. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
