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

feat(workflow_engine): Schedule Deletion for Workflow #85665

Merged
merged 3 commits into from
Feb 21, 2025

Conversation

saponifi3d
Copy link
Contributor

Description

Fix scheduled deletions for the Workflow model - this will now ensure that all the subsequent models are cleaned up correctly.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 21, 2025
@saponifi3d saponifi3d marked this pull request as ready for review February 21, 2025 18:21
@saponifi3d saponifi3d requested a review from a team as a code owner February 21, 2025 18:21
@saponifi3d saponifi3d requested a review from wedamija February 21, 2025 18:21
Copy link
Member

@ceorourke ceorourke left a comment

Choose a reason for hiding this comment

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

seems fine, left some non blocking comments

from sentry.testutils.helpers import TaskRunner
from sentry.testutils.hybrid_cloud import HybridCloudTestMixin

# from tests.sentry.workflow_engine.test_base import BaseWorkflowTest
Copy link
Member

Choose a reason for hiding this comment

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

guessing you meant to remove this

"action_condition",
],
)
def test_delete_workflow(self, instance_attr):
Copy link
Member

Choose a reason for hiding this comment

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

should we have a test with multiple actions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 also, woo! this comment made me realize i missed action in the test either 👀

@ameliahsu
Copy link
Member

Q: When an organization gets deleted, all of the organization's workflows get deleted. Will this task be triggered to clean up all of it's related models in that case?

I'm thinking about the problem we discussed last week about actions not getting deleted when the org is deleted.

@saponifi3d
Copy link
Contributor Author

saponifi3d commented Feb 21, 2025

Q: When an organization gets deleted, all of the organization's workflows get deleted. Will this task be triggered to clean up all of it's related models in that case?
I'm thinking about the problem we discussed last week about actions not getting deleted when the org is deleted.

@ameliahsu It doesn't seem like it -- locally, i added a test to confirm that the workflows are not cleaned up when you delete an org. we'd need to have a separate PR that's similar to this for the Organization

@saponifi3d saponifi3d enabled auto-merge (squash) February 21, 2025 23:31
@saponifi3d saponifi3d merged commit 8746111 into master Feb 21, 2025
49 checks passed
@saponifi3d saponifi3d deleted the jcallender/aci/workflow-delete branch February 21, 2025 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants