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/11 20:23:57 UTC
[GitHub] [qpid-dispatch] ganeshmurthy opened a new pull request #1212: DISPATCH-1930: Make sure log file is created before searching the log…
ganeshmurthy opened a new pull request #1212:
URL: https://github.com/apache/qpid-dispatch/pull/1212
… file
--
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 #1212: DISPATCH-1930: Make sure log file is created before searching the log…
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1212:
URL: https://github.com/apache/qpid-dispatch/pull/1212#discussion_r630520991
##########
File path: tests/system_tests_bad_configuration.py
##########
@@ -109,15 +109,23 @@ def wait_for_unresolvable_host(self):
then it stops scheduling new attempts.
:return:
"""
- with open(self.router.logfile_path, 'r') as router_log:
- log_lines = router_log.read().split("\n")
- expected_log_snip = "Connection to %s" % self.unresolvable_host_name
- errors_caught = [line for line in log_lines if expected_log_snip in line and "failed" in line]
-
- self.error_caught = any(errors_caught)
-
- # If condition not yet satisfied and not exhausted max attempts,
- # re-schedule the verification.
+ try:
+ with open(self.router.logfile_path, 'r') as router_log:
Review comment:
It's better to open file with 'rt', for Python 3 (and Windows :P)
--
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 #1212: DISPATCH-1930: Make sure log file is created before searching the log…
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1212:
URL: https://github.com/apache/qpid-dispatch/pull/1212#discussion_r630527031
##########
File path: tests/system_tests_bad_configuration.py
##########
@@ -109,15 +109,23 @@ def wait_for_unresolvable_host(self):
then it stops scheduling new attempts.
:return:
"""
- with open(self.router.logfile_path, 'r') as router_log:
- log_lines = router_log.read().split("\n")
- expected_log_snip = "Connection to %s" % self.unresolvable_host_name
- errors_caught = [line for line in log_lines if expected_log_snip in line and "failed" in line]
-
- self.error_caught = any(errors_caught)
-
- # If condition not yet satisfied and not exhausted max attempts,
- # re-schedule the verification.
+ try:
+ with open(self.router.logfile_path, 'r') as router_log:
Review comment:
+1, I knew 'r' is default, I did not know 't' is a default too.
--
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 closed pull request #1212: DISPATCH-1930: Make sure log file is created before searching the log…
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1212:
URL: https://github.com/apache/qpid-dispatch/pull/1212
--
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 #1212: DISPATCH-1930: Make sure log file is created before searching the log…
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #1212:
URL: https://github.com/apache/qpid-dispatch/pull/1212#discussion_r630522947
##########
File path: tests/system_tests_bad_configuration.py
##########
@@ -109,15 +109,23 @@ def wait_for_unresolvable_host(self):
then it stops scheduling new attempts.
:return:
"""
- with open(self.router.logfile_path, 'r') as router_log:
- log_lines = router_log.read().split("\n")
- expected_log_snip = "Connection to %s" % self.unresolvable_host_name
- errors_caught = [line for line in log_lines if expected_log_snip in line and "failed" in line]
-
- self.error_caught = any(errors_caught)
-
- # If condition not yet satisfied and not exhausted max attempts,
- # re-schedule the verification.
+ try:
+ with open(self.router.logfile_path, 'r') as router_log:
+ log_lines = router_log.read().split("\n")
+ expected_log_snip = "Connection to %s" % self.unresolvable_host_name
+ errors_caught = [line for line in log_lines if expected_log_snip in line and "failed" in line]
+
+ self.error_caught = any(errors_caught)
+
+ # If condition not yet satisfied and not exhausted max attempts,
+ # re-schedule the verification.
+ if self.waiting_for_error():
+ self.attempts_made += 1
+ self.schedule_timer()
+ except:
Review comment:
Will use IOError, thx
--
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 #1212: DISPATCH-1930: Make sure log file is created before searching the log…
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1212:
URL: https://github.com/apache/qpid-dispatch/pull/1212#issuecomment-839214203
# [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1212?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 [#1212](https://codecov.io/gh/apache/qpid-dispatch/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (aa3e100) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/2d11901a632525969d46a70ab2426f6e8695d686?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2d11901) will **decrease** coverage by `0.00%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/qpid-dispatch/pull/1212?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 #1212 +/- ##
==========================================
- Coverage 84.41% 84.40% -0.01%
==========================================
Files 113 113
Lines 28006 28005 -1
==========================================
- Hits 23642 23639 -3
- Misses 4364 4366 +2
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1212?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/adaptors/tcp\_adaptor.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1212/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=) | `69.89% <0.00%> (-0.36%)` | :arrow_down: |
| [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1212/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=) | `92.58% <0.00%> (-0.20%)` | :arrow_down: |
| [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1212/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=) | `89.63% <0.00%> (-0.10%)` | :arrow_down: |
| [src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1212/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=) | `93.34% <0.00%> (+0.17%)` | :arrow_up: |
| [src/parse.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1212/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.08% <0.00%> (+0.21%)` | :arrow_up: |
| [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1212/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=) | `94.18% <0.00%> (+0.43%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1212?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/1212?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 [2d11901...aa3e100](https://codecov.io/gh/apache/qpid-dispatch/pull/1212?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 #1212: DISPATCH-1930: Make sure log file is created before searching the log…
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #1212:
URL: https://github.com/apache/qpid-dispatch/pull/1212#discussion_r630525577
##########
File path: tests/system_tests_bad_configuration.py
##########
@@ -109,15 +109,23 @@ def wait_for_unresolvable_host(self):
then it stops scheduling new attempts.
:return:
"""
- with open(self.router.logfile_path, 'r') as router_log:
- log_lines = router_log.read().split("\n")
- expected_log_snip = "Connection to %s" % self.unresolvable_host_name
- errors_caught = [line for line in log_lines if expected_log_snip in line and "failed" in line]
-
- self.error_caught = any(errors_caught)
-
- # If condition not yet satisfied and not exhausted max attempts,
- # re-schedule the verification.
+ try:
+ with open(self.router.logfile_path, 'r') as router_log:
Review comment:
It says here that 't' is default - https://docs.python.org/3/library/functions.html#open
It also says that 'r' is default, so no need to specify any more in this case, it looks like
"mode is an optional string that specifies the mode in which the file is opened. It defaults to 'r' which means open for reading in text mode."
--
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 #1212: DISPATCH-1930: Make sure log file is created before searching the log…
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1212:
URL: https://github.com/apache/qpid-dispatch/pull/1212#discussion_r630519092
##########
File path: tests/system_tests_bad_configuration.py
##########
@@ -109,15 +109,23 @@ def wait_for_unresolvable_host(self):
then it stops scheduling new attempts.
:return:
"""
- with open(self.router.logfile_path, 'r') as router_log:
- log_lines = router_log.read().split("\n")
- expected_log_snip = "Connection to %s" % self.unresolvable_host_name
- errors_caught = [line for line in log_lines if expected_log_snip in line and "failed" in line]
-
- self.error_caught = any(errors_caught)
-
- # If condition not yet satisfied and not exhausted max attempts,
- # re-schedule the verification.
+ try:
+ with open(self.router.logfile_path, 'r') as router_log:
+ log_lines = router_log.read().split("\n")
+ expected_log_snip = "Connection to %s" % self.unresolvable_host_name
+ errors_caught = [line for line in log_lines if expected_log_snip in line and "failed" in line]
+
+ self.error_caught = any(errors_caught)
+
+ # If condition not yet satisfied and not exhausted max attempts,
+ # re-schedule the verification.
+ if self.waiting_for_error():
+ self.attempts_made += 1
+ self.schedule_timer()
+ except:
Review comment:
Bare `except` is very evil in Python, because it can catch absolutely any exception, including `NameError` or `SyntaxError`. It's better to put here `except IOError`, this works on Python 2 and 3 both. For Python 3.6 only, there could be `FileNotFoundError`. Death to Python 2! May it rest in peace.
--
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