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 2021/05/17 21:07:03 UTC
[GitHub] [qpid-dispatch] ganeshmurthy opened a new pull request #1221: DISPATCH-1986: Retry creating TestServer instance if port is already …
ganeshmurthy opened a new pull request #1221:
URL: https://github.com/apache/qpid-dispatch/pull/1221
…in use
--
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] codecov-commenter commented on pull request #1221: DISPATCH-1986: Retry creating TestServer instance if port is already …
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1221:
URL: https://github.com/apache/qpid-dispatch/pull/1221#issuecomment-843270481
# [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1221?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1221](https://codecov.io/gh/apache/qpid-dispatch/pull/1221?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1aba042) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/561306a4a9ae5952418713c497036f62156a90f7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (561306a) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/qpid-dispatch/pull/1221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## main #1221 +/- ##
==========================================
+ Coverage 84.60% 84.63% +0.02%
==========================================
Files 113 113
Lines 28008 28009 +1
==========================================
+ Hits 23697 23706 +9
+ Misses 4311 4303 -8
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1221/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `93.53% <0.00%> (-0.44%)` | :arrow_down: |
| [src/router\_core/forwarder.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1221/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2ZvcndhcmRlci5j) | `92.64% <0.00%> (-0.40%)` | :arrow_down: |
| [src/adaptors/http1/http1\_server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1221/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL2h0dHAxL2h0dHAxX3NlcnZlci5j) | `84.57% <0.00%> (-0.30%)` | :arrow_down: |
| [src/parse.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1221/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3BhcnNlLmM=) | `87.96% <0.00%> (-0.20%)` | :arrow_down: |
| [src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1221/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2l0ZXJhdG9yLmM=) | `89.29% <0.00%> (-0.19%)` | :arrow_down: |
| [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1221/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=) | `93.66% <0.00%> (+0.09%)` | :arrow_up: |
| [src/router\_core/delivery.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1221/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2RlbGl2ZXJ5LmM=) | `93.51% <0.00%> (+0.18%)` | :arrow_up: |
| [src/adaptors/tcp\_adaptor.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1221/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL3RjcF9hZGFwdG9yLmM=) | `75.49% <0.00%> (+0.46%)` | :arrow_up: |
| [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1221/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `90.52% <0.00%> (+0.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1221?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1221?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [561306a...1aba042](https://codecov.io/gh/apache/qpid-dispatch/pull/1221?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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 a change in pull request #1221: DISPATCH-1986: Retry creating TestServer instance if port is already …
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #1221:
URL: https://github.com/apache/qpid-dispatch/pull/1221#discussion_r634381208
##########
File path: tests/http1_tests.py
##########
@@ -272,13 +273,11 @@ def http1_ping(sport, cport):
]
}
- server = TestServer(server_port=sport,
- client_port=cport,
- tests=TEST)
+ server = wait_server_starting(sport, cport, TEST)
Review comment:
The tests bring up and bring down the TestServer (in some cases 2 test servers). The TestServer(s) seem to shut down successfully but the port is not released on time before the next test comes along and starts the next TestServer on the same port(s). This fix will wait for the few seconds until the port is released. Could take more than a few seconds for the port to be released but that is something I have not seen yet.
--
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 #1221: DISPATCH-1986: Retry creating TestServer instance if port is already …
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1221:
URL: https://github.com/apache/qpid-dispatch/pull/1221#discussion_r634392753
##########
File path: tests/http1_tests.py
##########
@@ -367,6 +366,27 @@ def check_response(self, rsp):
return body
+def wait_server_starting(server_port, client_port, tests, handler_cls=None):
Review comment:
This is a factory method. I would suggest a more "pythonic" approach would make this a class method of the TestServer class and call it everywhere we need to instantiate a TestServer.
--
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 a change in pull request #1221: DISPATCH-1986: Retry creating TestServer instance if port is already …
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1221:
URL: https://github.com/apache/qpid-dispatch/pull/1221#discussion_r634199477
##########
File path: tests/http1_tests.py
##########
@@ -272,13 +273,11 @@ def http1_ping(sport, cport):
]
}
- server = TestServer(server_port=sport,
- client_port=cport,
- tests=TEST)
+ server = wait_server_starting(sport, cport, TEST)
Review comment:
This problem would go away entirely if the test could start the server first, before routers, with `TestServer(server_port=0, ...` and then queried the server for an actual port when starting the routers. But if the approach of the PR works reliably, then I see no reason to do yet another change to 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] jiridanek commented on a change in pull request #1221: DISPATCH-1986: Retry creating TestServer instance if port is already …
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1221:
URL: https://github.com/apache/qpid-dispatch/pull/1221#discussion_r634199477
##########
File path: tests/http1_tests.py
##########
@@ -272,13 +273,11 @@ def http1_ping(sport, cport):
]
}
- server = TestServer(server_port=sport,
- client_port=cport,
- tests=TEST)
+ server = wait_server_starting(sport, cport, TEST)
Review comment:
This problem would go away entirely if the test started the server first, before routers, with `TestServer(server_port=0, ...` and then queried the server for an actual port when starting the routers. But if the approach of the PR works reliably, then I see no reason to do yet another change to 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] asfgit merged pull request #1221: DISPATCH-1986: Retry creating TestServer instance if port is already …
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #1221:
URL: https://github.com/apache/qpid-dispatch/pull/1221
--
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