-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
tests(deletions): Add CustomTaskQueue to enable testing #103974
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
Conversation
| # Configure within each Process | ||
| import logging | ||
|
|
||
| from sentry.utils.imports import import_string |
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.
Some of the logic from the multiprocess_worker function is moved into a new function called task_execution.
This allows testing the deletion without being impacted by the multiprocessing set up (DB changes by the tests are not seen by the processes).
| ) | ||
|
|
||
| for chunk in q.iterator(chunk_size=100): | ||
| for chunk in q.iterator(chunk_size=DELETES_BY_PROJECT_CHUNK_SIZE): |
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.
This will allow patching within the test.
|
|
||
| with ( | ||
| assume_test_silo_mode(SiloMode.REGION), | ||
| patch("sentry.runner.commands.cleanup.DELETES_BY_PROJECT_CHUNK_SIZE", 2), |
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.
Two groups at a time will be processed.
| def test_run_bulk_query_deletes_by_project(self) -> None: | ||
| """Test that the function runs bulk query deletes by project as expected.""" | ||
| days = 30 | ||
| self.create_group() |
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 only group that won't be past retention.
The changes includes allow replacing the task_queue with a custom class to support testing of cleanup functions.
4613654 to
b0a31a6
Compare
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
yuvmen
left a comment
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.
nice! just handle the few bot comments I think they could be right
The changes includes allow replacing the task_queue with a custom class to support testing of cleanup functions.