[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_r232695625
##
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:
@nickva Exactly the same logic as in my implementation is used in the
`db_doc_req`
[here](https://github.com/apache/couchdb/blob/master/src/chttpd/src/chttpd_db.erl#L756)
where the check is done only for `multipart` `Accept` header. When i wrote
this part of `_bulk_get` logic I mostly copied `db_doc_req`.
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_r232638898
##
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:
`MochiWeb`'s `accepts_content_type("application/json")` returns `true` and
multipart case clause is not executed, even though the client (couchbase-lite)
sends these headers:
```
Accept = "multipart/related";
"Content-Type" = "application/json";
```
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_r232632644
##
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:
@nickva Actually this change broke my 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 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_r232695625
##
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:
@nickva Also exactly the same logic as in my implementation is used in the
`db_doc_req`
[here](https://github.com/apache/couchdb/blob/master/src/chttpd/src/chttpd_db.erl#L756)
where the check is done only for `multipart` `Accept` header. When i wrote
this part of `_bulk_get` logic I mostly copied `db_doc_req`.
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_r232695625
##
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:
@nickva Also exactly the same logic as in my implementation is used in the
`db_doc_req`
[here](https://github.com/apache/couchdb/blob/master/src/chttpd/src/chttpd_db.erl#L756)
where the check is done only for `multipart` `Accept` header. When i wrote my
implementation for multipart `_bulk_get` I mostly copied `db_doc_req` logic.
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_r232695625
##
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:
@nickva Also I the same logic as in my implementation is used in
`db_doc_req`
[here](https://github.com/apache/couchdb/blob/master/src/chttpd/src/chttpd_db.erl#L756)
where the check is done only for `multipart` `Accept` header. When i wrote my
implementation for multipart `_bulk_get` I mostly copied `db_doc_req` logic.
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_r232695625
##
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:
@nickva Also I the same logic as in my implementation is used in
`db_doc_req`
[here](https://github.com/apache/couchdb/blob/master/src/chttpd/src/chttpd_db.erl#L756)
where the first check is for `multipart` `Accept` header. When i wrote this
implementation for `_bulk_get` I mostly copied `db_doc_req` logic.
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_r232695625
##
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:
@nickva Also I see the same logic in the normal db/doc request
[here](https://github.com/apache/couchdb/blob/master/src/chttpd/src/chttpd_db.erl#L756)
where we first just check for `multipart` `Accept` header. When i wrote this
implementation for `_bulk_get` I mostly copied `db_doc_req` logic.
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_r232660021
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -997,6 +1075,23 @@ send_docs_multipart(Req, Results, Options1) ->
couch_httpd:send_chunk(Resp, <<"--">>),
couch_httpd:last_chunk(Resp).
+bulk_get_multipart_headers({0, []}, Id, Boundary) ->
+[
+<<"\r\nX-Doc-Id: ", Id/binary>>,
+<<"\r\nContent-Type: multipart/related; boundary=", Boundary/binary,
"\r\n\r\n">>
+];
+bulk_get_multipart_headers({Start, [FirstRevId|_]}, Id, Boundary) ->
+RevStr = couch_doc:rev_to_str({Start, FirstRevId}),
+[
+<<"\r\nX-Doc-Id: ", Id/binary>>,
+<<"\r\nX-Rev-Id: ", RevStr/binary>>,
+<<"\r\nContent-Type: multipart/related; boundary=", Boundary/binary,
"\r\n\r\n">>
+].
+
+bulk_get_multipart_boundary() ->
+Unique = couch_uuids:random(),
+<<"---", Unique/binary>>.
Review comment:
> Was it for testing mainly
Exactly. I've just forgotten to clean this up. Committed a fix.
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_r232638898
##
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:
`MochiWeb`'s `accepts_content_type("application/json")` returns `true` and
multipart case clause is not executed, even though the client (couchbase-lite)
sends these headers:
```
Accept = "multipart/related";
"Content-Type" = "application/json";
```
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_r232638898
##
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:
`MochiWeb`'s `accepts_content_type("application/json")` returns `true` even
though the client (couchbase-lite) sends these headers:
```
Accept = "multipart/related";
"Content-Type" = "application/json";
```
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_r232632644
##
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:
@nickva Actually this change broke my 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 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_r232632644
##
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:
@nickva Actually this change broke my tests with. Have to investigate why.
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_r232638898
##
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:
For some reason `MochiWeb`'s `accepts_content_type("application/json")`
returns `true` even though the client sent these headers:
```
Accept = "multipart/related";
"Content-Type" = "application/json";
```
With previous logic it works perfectly because we have an explicit check for
multipart `Accept`.
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_r232632644
##
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:
@nickva Actually this change broke my tests with `got invalid Content-Type
'application/json'`
Have to investigate why.
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_r232581368
##
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:
@nickva Valid point even though I consider this kind of scenario a rare edge
case. I'll make appropriate 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
[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_r232620377
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -997,6 +1075,23 @@ send_docs_multipart(Req, Results, Options1) ->
couch_httpd:send_chunk(Resp, <<"--">>),
couch_httpd:last_chunk(Resp).
+bulk_get_multipart_headers({0, []}, Id, Boundary) ->
+[
+<<"\r\nX-Doc-Id: ", Id/binary>>,
+<<"\r\nContent-Type: multipart/related; boundary=", Boundary/binary,
"\r\n\r\n">>
+];
+bulk_get_multipart_headers({Start, [FirstRevId|_]}, Id, Boundary) ->
+RevStr = couch_doc:rev_to_str({Start, FirstRevId}),
+[
+<<"\r\nX-Doc-Id: ", Id/binary>>,
+<<"\r\nX-Rev-Id: ", RevStr/binary>>,
+<<"\r\nContent-Type: multipart/related; boundary=", Boundary/binary,
"\r\n\r\n">>
+].
+
+bulk_get_multipart_boundary() ->
+Unique = couch_uuids:random(),
+<<"---", Unique/binary>>.
Review comment:
> Was it for testing mainly
Exactly. Will clean it up.
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_r232620377
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -997,6 +1075,23 @@ send_docs_multipart(Req, Results, Options1) ->
couch_httpd:send_chunk(Resp, <<"--">>),
couch_httpd:last_chunk(Resp).
+bulk_get_multipart_headers({0, []}, Id, Boundary) ->
+[
+<<"\r\nX-Doc-Id: ", Id/binary>>,
+<<"\r\nContent-Type: multipart/related; boundary=", Boundary/binary,
"\r\n\r\n">>
+];
+bulk_get_multipart_headers({Start, [FirstRevId|_]}, Id, Boundary) ->
+RevStr = couch_doc:rev_to_str({Start, FirstRevId}),
+[
+<<"\r\nX-Doc-Id: ", Id/binary>>,
+<<"\r\nX-Rev-Id: ", RevStr/binary>>,
+<<"\r\nContent-Type: multipart/related; boundary=", Boundary/binary,
"\r\n\r\n">>
+].
+
+bulk_get_multipart_boundary() ->
+Unique = couch_uuids:random(),
+<<"---", Unique/binary>>.
Review comment:
> Was it for testing mainly
Exactly. Will clean it up.
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_r232581368
##
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:
@nickva Valid point even though I consider this kind of scenario a rare edge
case. I'll make appropriate 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
[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] 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] 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_r232008802
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -932,7 +973,39 @@ send_doc_efficiently(#httpd{mochi_req=MochiReq}=Req,
#doc{atts=Atts}=Doc, Header
send_json(Req, 200, Headers, couch_doc:to_json_obj(Doc, Options))
end.
-send_docs_multipart(Req, Results, Options1) ->
+send_docs_multipart(bulk_get, _Req, Results, Options1, OuterBoundary, Resp) ->
Review comment:
Hi @nickva and thanks for review and your comments. I will definitely
address this. I should have thought about fixing all this formatting mess
before. One question for you: in your opinion is it OK to have
`send_docs_multipart` reimplemented as a function clause tagged with atom
`bulk_get`? If it's _clearer_ to reimplement this as a separate 3 param
function then I will definitely do this.
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_r232011648
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -932,7 +973,39 @@ send_doc_efficiently(#httpd{mochi_req=MochiReq}=Req,
#doc{atts=Atts}=Doc, Header
send_json(Req, 200, Headers, couch_doc:to_json_obj(Doc, Options))
end.
-send_docs_multipart(Req, Results, Options1) ->
+send_docs_multipart(bulk_get, _Req, Results, Options1, OuterBoundary, Resp) ->
Review comment:
Sorry, overlooked this comment when I had written my questions above. Sure,
will do.
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_r232009223
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -932,7 +973,39 @@ send_doc_efficiently(#httpd{mochi_req=MochiReq}=Req,
#doc{atts=Atts}=Doc, Header
send_json(Req, 200, Headers, couch_doc:to_json_obj(Doc, Options))
end.
-send_docs_multipart(Req, Results, Options1) ->
+send_docs_multipart(bulk_get, _Req, Results, Options1, OuterBoundary, Resp) ->
Review comment:
Also a second question. Shall I submit all post code review changes here as
a separate commit or perhaps it would be better/cleaner to create a new PR from
master with single commit. I'm open to all proposals.
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_r232011648
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -932,7 +973,39 @@ send_doc_efficiently(#httpd{mochi_req=MochiReq}=Req,
#doc{atts=Atts}=Doc, Header
send_json(Req, 200, Headers, couch_doc:to_json_obj(Doc, Options))
end.
-send_docs_multipart(Req, Results, Options1) ->
+send_docs_multipart(bulk_get, _Req, Results, Options1, OuterBoundary, Resp) ->
Review comment:
Sorry, overlooked this comment when I had written my questions above. Sure
will do.
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_r232011648
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -932,7 +973,39 @@ send_doc_efficiently(#httpd{mochi_req=MochiReq}=Req,
#doc{atts=Atts}=Doc, Header
send_json(Req, 200, Headers, couch_doc:to_json_obj(Doc, Options))
end.
-send_docs_multipart(Req, Results, Options1) ->
+send_docs_multipart(bulk_get, _Req, Results, Options1, OuterBoundary, Resp) ->
Review comment:
Sorry overlooked this comment when I had written my questions above. Sure
will do.
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_r232008802
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -932,7 +973,39 @@ send_doc_efficiently(#httpd{mochi_req=MochiReq}=Req,
#doc{atts=Atts}=Doc, Header
send_json(Req, 200, Headers, couch_doc:to_json_obj(Doc, Options))
end.
-send_docs_multipart(Req, Results, Options1) ->
+send_docs_multipart(bulk_get, _Req, Results, Options1, OuterBoundary, Resp) ->
Review comment:
Hi @nickva and thanks for review and your comments. Will definitely address
this. I should have thought about fixing all this formatting mess before. One
question for you: in your opinion is it OK to have `send_docs_multipart`
reimplemented as a separate function clause tagged with atom `bulk_get`? If
it's _clearer_ to reimplement this as a separate 3 param function then i will
definitely do this.
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_r232009223
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -932,7 +973,39 @@ send_doc_efficiently(#httpd{mochi_req=MochiReq}=Req,
#doc{atts=Atts}=Doc, Header
send_json(Req, 200, Headers, couch_doc:to_json_obj(Doc, Options))
end.
-send_docs_multipart(Req, Results, Options1) ->
+send_docs_multipart(bulk_get, _Req, Results, Options1, OuterBoundary, Resp) ->
Review comment:
Also a second question. Shall I submit all post code review changes here as
a separate commit or perhaps it would be better/cleaner to create a new PR from
master with single commit. I'm opened to all propositions.
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_r232008802
##
File path: src/chttpd/src/chttpd_db.erl
##
@@ -932,7 +973,39 @@ send_doc_efficiently(#httpd{mochi_req=MochiReq}=Req,
#doc{atts=Atts}=Doc, Header
send_json(Req, 200, Headers, couch_doc:to_json_obj(Doc, Options))
end.
-send_docs_multipart(Req, Results, Options1) ->
+send_docs_multipart(bulk_get, _Req, Results, Options1, OuterBoundary, Resp) ->
Review comment:
Hi @nickva and thanks for review and your comments. Will definitely address
this. I should have thought about fixing all this formatting mess before. One
question for you: in your opinion is it OK to have `send_docs_multipart`
reimplemented as a separate function clause tagged with atom `bulk_get`? If
it's _clearer_ to reimplement this as a separate 3 param function then i will
definitely address this.
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
