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

Make sure checkins from crons work and keep working #3155

Conversation

antonpirker
Copy link
Member

@antonpirker antonpirker commented Jun 11, 2024

After refactoring the Celery Beat integration there was a regression, where the ok/error check ins for crons where not sent.

We reverted the refactoring here: #3144

This PR:

After we have merged this we can revert the revert linked above.

Fixes #3145

@antonpirker antonpirker self-assigned this Jun 11, 2024
@antonpirker antonpirker force-pushed the antonpirker/fix-crons-checkin-on-success branch from 1a2e690 to 7bf044c Compare June 11, 2024 15:33
@@ -26,9 +26,19 @@ def inner(signal, f):

@pytest.fixture
def init_celery(sentry_init, request):
def inner(propagate_traces=True, backend="always_eager", **kwargs):
def inner(
Copy link
Member Author

Choose a reason for hiding this comment

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

I updated this init_celery to make sure we can also set monitor_beat_tasks here.

@antonpirker
Copy link
Member Author

We have now the old init_celery fixture and a new celery_init fixture.

The new one is way easier, and I think the old one is not needed anymore because advancements in Celery. So I think we can move to the new simpler fixture to be used everywhere.

Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

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

Looking good. In the longer term we should look into moving the old tests to the new setup too.

sentry_sdk/integrations/celery/beat.py Outdated Show resolved Hide resolved
@antonpirker antonpirker changed the base branch from master to antonpirker/fix-celery-beat-refactoring June 17, 2024 14:34
@antonpirker antonpirker changed the base branch from antonpirker/fix-celery-beat-refactoring to master June 17, 2024 14:37
@antonpirker antonpirker changed the base branch from master to antonpirker/fix-celery-beat-refactoring June 17, 2024 14:57
@antonpirker
Copy link
Member Author

Closing in favor of #3176

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.

Test Celery Beat regression (2.4.0 through 2.5.0 inclusive)
2 participants