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.

Sign up for GitHub

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

Jump to bottom

storage endpoint for deleting backfills #25165

Closed
wants to merge 13 commits into from
Closed

storage endpoint for deleting backfills #25165

wants to merge 13 commits into from

Conversation

jamiedemaria
Copy link
Contributor

@jamiedemaria jamiedemaria commented Oct 9, 2024 •
edited
Loading

Summary & Motivation

adds storage endpoint for deleting backfills. This will NOT delete any runs that the backfill launched, just deletes the row for the backfill from the BulkActionsTable

associated internal pr https://github.com/dagster-io/internal/pull/12090

How I Tested These Changes

new unit tests

@jamiedemaria Graphite App
Copy link
Contributor Author

jamiedemaria commented Oct 9, 2024 •
edited
Loading

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @jamiedemaria and the rest of your teammates on Graphite Graphite

This was referenced Oct 9, 2024
Merged
Closed
Closed
Closed
@jamiedemaria jamiedemaria force-pushed the jamie/storage-delete-backfill branch from c3f0015 to 13547e1 Compare October 9, 2024 20:48
@jamiedemaria jamiedemaria changed the base branch from jamie/backfill-retries to jamie/delete-mmultiple-runs October 9, 2024 20:48
@jamiedemaria jamiedemaria changed the title add delete backfill to storage storage endpoint for deleting backfills Oct 9, 2024
jamiedemaria
jamiedemaria commented Oct 9, 2024
View reviewed changes
python_modules/dagster/dagster/_core/storage/runs/sql_run_storage.py
@@ -1023,6 +1023,14 @@ def update_backfill(self, partition_backfill: PartitionBackfill) -> None:
)
)

def delete_backfill(self, backfill_id: str) -> None:
check.str_param(backfill_id, "backfill_id")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

look at putting this into a single transaction

@jamiedemaria jamiedemaria force-pushed the jamie/delete-mmultiple-runs branch from b5ac4d7 to 2b17776 Compare October 10, 2024 15:05
@jamiedemaria jamiedemaria force-pushed the jamie/storage-delete-backfill branch from 13547e1 to 79bff43 Compare October 10, 2024 15:05
@jamiedemaria jamiedemaria force-pushed the jamie/delete-mmultiple-runs branch from 2b17776 to 00b56ce Compare October 10, 2024 16:45
@jamiedemaria jamiedemaria force-pushed the jamie/storage-delete-backfill branch from 79bff43 to 1f64831 Compare October 10, 2024 16:45
@jamiedemaria jamiedemaria force-pushed the jamie/delete-mmultiple-runs branch from 00b56ce to f893974 Compare October 11, 2024 13:39
@jamiedemaria jamiedemaria force-pushed the jamie/storage-delete-backfill branch from 1f64831 to dbd2468 Compare October 11, 2024 13:39
@jamiedemaria jamiedemaria marked this pull request as ready for review October 11, 2024 13:42
@krukowski
Copy link
Contributor

Call out from @gibsondan on a separate issue that's worth considering here:

Wiping an asset does a giant delete on a bunch of derived tables, which can fail. @prha has had to manually wipe assets for [customer], and I could definitely imagine other orgs running into timeouts. We probably need some kind of chunking or bulk action here instead of trying to do it all in one big delete.

@jamiedemaria
Copy link
Contributor Author

popping this back in draft to get out of queues until we figure out a deleting runs strategy #25167 (review)

@jamiedemaria jamiedemaria reopened this Oct 11, 2024
@jamiedemaria jamiedemaria marked this pull request as draft October 11, 2024 14:59
@jamiedemaria jamiedemaria reopened this Oct 14, 2024
@jamiedemaria jamiedemaria force-pushed the jamie/storage-delete-backfill branch from dbd2468 to a0ba58c Compare October 14, 2024 13:56
@jamiedemaria jamiedemaria changed the base branch from jamie/delete-mmultiple-runs to jamie/backfill-retries October 14, 2024 13:56
@github-actions GitHub Actions
Copy link

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-mss8gxsvb-elementl.vercel.app
https://jamie-storage-delete-backfill.core-storybook.dagster-docs.io

Built with commit a0ba58c.
This pull request is being automatically deployed with vercel-action

@github-actions GitHub Actions
Copy link

Deploy preview for dagit-storybook ready!

✅ Preview
https://dagit-storybook-mmlx7a82u-elementl.vercel.app
https://jamie-storage-delete-backfill.components-storybook.dagster-docs.io

Built with commit a0ba58c.
This pull request is being automatically deployed with vercel-action

@github-actions GitHub Actions
Copy link

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-mojehito1-elementl.vercel.app
https://jamie-storage-delete-backfill.dagster.dagster-docs.io

Direct link to changed pages:

@github-actions GitHub Actions
Copy link

Deploy preview for dagster-university ready!

✅ Preview
https://dagster-university-eydbiny0j-elementl.vercel.app
https://jamie-storage-delete-backfill.dagster-university.dagster-docs.io

Built with commit a0ba58c.
This pull request is being automatically deployed with vercel-action

@github-actions GitHub Actions
Copy link

Deploy preview for dagster-docs-beta ready!

Preview available at https://dagster-docs-beta-fpyqo0ald-elementl.vercel.app

Direct link to changed pages:

@jamiedemaria jamiedemaria mentioned this pull request Oct 14, 2024
Closed
@jamiedemaria jamiedemaria force-pushed the jamie/storage-delete-backfill branch 3 times, most recently from 5f35ff8 to 0d4ead6 Compare October 15, 2024 13:15
@jamiedemaria jamiedemaria force-pushed the jamie/backfill-retries branch from 3cd9f4a to 58456f5 Compare October 15, 2024 13:45
@jamiedemaria jamiedemaria force-pushed the jamie/storage-delete-backfill branch from 0d4ead6 to b69d197 Compare October 15, 2024 13:45
Base automatically changed from jamie/backfill-retries to master October 16, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jamiedemaria @krukowski