-
-
Notifications
You must be signed in to change notification settings - Fork 720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove dump cluster from gen_cluster #8823
Remove dump cluster from gen_cluster #8823
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 25 files ± 0 25 suites ±0 10h 9m 33s ⏱️ - 6m 10s For more details on these failures, see this check. Results for commit f97062d. ± Comparison against base commit 3075b08. This pull request removes 6 tests.
♻️ This comment has been updated with latest results. |
CI isn't happy with this PR, need help fixing this up? |
6940a3d
to
0173ffb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @fjetter. I left one non-blocking comment but otherwise this LGTM
@@ -1122,41 +1106,6 @@ async def async_fn_outer(): | |||
return _ | |||
|
|||
|
|||
async def dump_cluster_state( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option is to leave this functionality around but still disable it in gen_cluster
. I'm not sure how much value there is to having dump_cluster_state
around for diagnosing tricky deadlock situations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The functionality itself is part of Client. I'm not entirely sure what this utility function offers but I'd rather not maintain it.
I don't see a lot of value for having this still around. However, it is spamming the working directory and makes test execution (if failed) much slower.