You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2020/10/20 19:56:51 UTC
[GitHub] [qpid-dispatch] grs opened a new pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
grs opened a new pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] kgiusti commented on a change in pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#discussion_r510304631
##########
File path: src/adaptors/http1/http1_client.c
##########
@@ -722,6 +724,17 @@ static int _client_rx_headers_done_cb(h1_codec_request_state_t *hrs, bool has_bo
qd_compose_insert_string(props, hconn->cfg.address); // to
qd_compose_insert_string(props, h1_codec_request_state_method(hrs)); // subject
qd_compose_insert_string(props, hconn->client.reply_to_addr); // reply-to
+ qd_compose_insert_null(props); // correlation-id
+ qd_compose_insert_null(props); // content-type
+ qd_compose_insert_null(props); // content-encoding
+ qd_compose_insert_null(props); // absolute-expiry-time
+ qd_compose_insert_null(props); // creation-time
+ qd_compose_insert_string(props, hconn->cfg.site); // group-id
+
+ assert(hconn->client.reply_to_addr && strlen(hconn->client.reply_to_addr)); //remove me
Review comment:
Is this intended for inclusion with this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] ganeshmurthy commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-715471623
@grs the http2 system tests you have added look good to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] grs commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
grs commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-713158403
@kgiusti @ganeshmurthy
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] ganeshmurthy commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-715594517
> @kgiusti @ganeshmurthy if there are no objections I will merge this to the dev-protocol-adapaters-2 branch on Monday.
Please go right ahead. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] asfgit merged pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-713415675
@grs The logs look a lot like what's currently on the dev-protocol-adaptors-2 branch, so you're most likely right.
I admit I am not following the branch closely; my interest is in making sure the CI is not broken due to missing dependencies (as it was at one point), or due to insufficiently configured networking, as the GHA might be in the future. If the CI becomes irrelevant at any point, people will stop watching it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] grs edited a comment on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
grs edited a comment on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-714786399
@kgiusti I have added tests for http2 stats and will add for http1 once the existing http1 tests are passing (so I can reuse your setup etc)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] grs commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
grs commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-713411740
@jiridanek that test is failing independent of this PR though I believe, and I think @kgiusti is looking into it? Or is this PR introducing a new failure?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] grs commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
grs commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-715564861
@jiridanek your concern was justified, there was indeed an issue that caused additional test failures. Now fixed though and seems to be similar to the errors on clean dev branch.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-713384242
I was a bit concerned about the system_tests_http1_adaptor failing in GH Action with
Exception: wait_port timeout on host None port 23906: [Errno 111] Connection refused
because that might've been caused by the cgroups network sandbox that the Actions enable, but there is the same test failing on Travis as well, where the sandbox is not used. So it's probably not environmental.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] grs commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
grs commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-715565210
@kgiusti @ganeshmurthy if there are no objections I will merge this to the dev-protocol-adapaters-2 branch on Monday.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] grs commented on a change in pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
grs commented on a change in pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#discussion_r510337023
##########
File path: src/adaptors/http1/http1_client.c
##########
@@ -722,6 +724,17 @@ static int _client_rx_headers_done_cb(h1_codec_request_state_t *hrs, bool has_bo
qd_compose_insert_string(props, hconn->cfg.address); // to
qd_compose_insert_string(props, h1_codec_request_state_method(hrs)); // subject
qd_compose_insert_string(props, hconn->client.reply_to_addr); // reply-to
+ qd_compose_insert_null(props); // correlation-id
+ qd_compose_insert_null(props); // content-type
+ qd_compose_insert_null(props); // content-encoding
+ qd_compose_insert_null(props); // absolute-expiry-time
+ qd_compose_insert_null(props); // creation-time
+ qd_compose_insert_string(props, hconn->cfg.site); // group-id
+
+ assert(hconn->client.reply_to_addr && strlen(hconn->client.reply_to_addr)); //remove me
Review comment:
Not the assert! I must have accidentally included that when merging conflicts in the rebase. Will remove.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] grs commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
grs commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-714786399
@kgiusti I have added a test for http2 stats and will add for http1 once the existing http1 tests are passing (so I can reuse your setup etc)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] grs commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
grs commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-713423417
Agreed. (And it did catch one issue specific to this PR, which I fixed.)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] kgiusti commented on pull request #888: DISPATCH-1779: initial http stats for http1 and http2 adaptors
Posted by GitBox <gi...@apache.org>.
kgiusti commented on pull request #888:
URL: https://github.com/apache/qpid-dispatch/pull/888#issuecomment-714622626
@jiridanek @grs
The HTTP1 system tests are not passing for a variety of issues that have nothing to do with this patch.
I'm working towards resolving them. This patch should land regardless.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org