Allow heartbeats to be sent in tests

[danihodovic]

I'm writing a Prometheus exporter at
https://github.com/danihodovic/celery-exporter.

In order to test the worker events: worker-heartbeat, worker-online,
worker-offline I need to be able to enable heartbeats for the testing
worker.

The change allows for overriding the default value of heartbeat = False.

[lgtm-com]

This pull request fixes 1 alert when merging b6376da into 948bb79 - view on LGTM.com

fixed alerts:

• 1 for Non-exception in 'except' clause

[auvipy]

this isn't going to break the existing API write? shouldn't this overridable feature mentioned in the docs? and can we provide some sort of unit tests?

[danihodovic]

The default value is True, so this should be backwards compatible.

[auvipy]

can you add documentation for this?

[danihodovic]

Yes, probably next week.

[thedrow]

This solution means you'll need to overwrite the celery_worker/celery_session_worker fixtures to override the value of without_heartbeat.
This isn't the most comfortable API.

We should change https://github.com/celery/celery/blob/master/celery/contrib/pytest.py#L145 instead to return {"without_heartbeat": True} and ensure it is passed onwards.

[danihodovic]

We should change https://github.com/celery/celery/blob/master/celery/contrib/pytest.py#L145 instead to return {"without_heartbeat": True} and ensure it is passed onwards.

The question is if this breaks existing behavior for users that don't use the pytest fixture? By default the value is False -

celery/celery/worker/consumer/heart.py

Line 22 in 7288147

without_heartbeat=False, heartbeat_interval=None, **kwargs):

.

WorkController behavior would silently change and heartsbeats would be enabled.

This solution means you'll need to overwrite the celery_worker/celery_session_worker fixtures to override the value of without_heartbeat.

No, you can override celery_worker_parameters as seen here:
https://github.com/danihodovic/celery-exporter/blob/e5430516de22b9f095da4d8d099169ec518b6670/conftest.py#L15-L18

[thedrow]

We should change https://github.com/celery/celery/blob/master/celery/contrib/pytest.py#L145 instead to return {"without_heartbeat": True} and ensure it is passed onwards.

The question is if this breaks existing behavior for users that don't use the pytest fixture? By default the value is False -

celery/celery/worker/consumer/heart.py

Line 22 in 7288147

without_heartbeat=False, heartbeat_interval=None, **kwargs):

.

WorkController behavior would silently change and heartsbeats would be enabled.

In that case we still need to check if celery_worker_parameters contains the without_heartbeat key and if it does not, we need to add it ourselves.

This solution means you'll need to overwrite the celery_worker/celery_session_worker fixtures to override the value of without_heartbeat.

No, you can override celery_worker_parameters as seen here:
https://github.com/danihodovic/celery-exporter/blob/e5430516de22b9f095da4d8d099169ec518b6670/conftest.py#L15-L18

I meant the solution proposed in this PR, not the solution I'm proposing.

[danihodovic]

In that case we still need to check if celery_worker_parameters contains the without_heartbeat key and if it does not, we need to add it ourselves.

Hmm... I'm confused because that's what this PR does https://github.com/celery/celery/pull/6632/files#diff-fee53e249a978fd480b1cade781b4e997b18492ab090d76d0377786d99128b92R122

kwargs in _start_worker_thread is passed in from celery_worker_parameters

• celery/celery/contrib/testing/worker.py

  Line 99 in b6376da

  **kwargs):

• celery/celery/contrib/pytest.py

  Line 196 in b6376da

  **celery_worker_parameters) as w:

[thedrow]

Gottcha 🤦
The lack of documentation (that's our fault) confused me.

[danihodovic]

Gottcha facepalm
The lack of documentation (that's our fault) confused me.

No worries, happy we cleared it up. I'll be adding some docs later this week.

[lgtm-com]

This pull request introduces 1 alert and fixes 1 when merging 412d576 into 1eade4c - view on LGTM.com

new alerts:

• 1 for Module is imported with 'import' and 'import from'

fixed alerts:

• 1 for Non-exception in 'except' clause

[auvipy]

thanks!