Skip to content
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

enhance: allow users to disable broker heartbeats #1998

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

smart-programmer
Copy link

broker heartbeats clutter the logs console which prevents users from seeing important logs. this allows users to disable broker heartbeats if timeout isn't provided.

reference issue

Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change seems feasible, however, is it possible for you to add some additional tests for the change?

@FrankK-1234
Copy link
Contributor

Hello,

I took the liberty to add a test case for this change. However I'm failing to add to a pull request as I'm new to git and I get a permission denied pushing to smart-programmer branch. No clue how to do it. I however attached a patch file to add the change in t/unit/test_mixins.py. Would that test case be enough for the proposed change? (I somehow cannot upload .patch so used .txt instead)
test_mixins.txt

@auvipy
Copy link
Member

auvipy commented May 30, 2024

thanks for the test! you have to open a PR on smart-programmer:enhance/disable-heartbeat-no-timeout branch. if you can't do that we will add the test from your provided code!

@auvipy
Copy link
Member

auvipy commented May 30, 2024

Hello,

I took the liberty to add a test case for this change. However I'm failing to add to a pull request as I'm new to git and I get a permission denied pushing to smart-programmer branch. No clue how to do it. I however attached a patch file to add the change in t/unit/test_mixins.py. Would that test case be enough for the proposed change? (I somehow cannot upload .patch so used .txt instead) test_mixins.txt

or you can open a full PR with your tests and this PR's changes as well! we wil make sure both will get contributors credit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants