[dagster-duckdb] gracefully handle table cleanup exceptions on views #26756
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OneCyrus
changed the title
OneCyrus
changed the title
jamiedemaria
requested changes
Dec 31, 2024
| @@ -262,6 +262,14 @@ def delete_table_slice(context: OutputContext, table_slice: TableSlice, connecti | |||
| except duckdb.CatalogException: | |||
| # table doesn't exist yet, so ignore the error | |||
| pass | |||
| except duckdb.BinderException as ex: | |||
There was a problem hiding this comment.
I think adding a test for this behavior would be helpful! That way if there are changes in the duckdb package that break this path we'll find out sooner. I think the easiest place for you to put this would be in python_modules/libraries/dagster-duckdb-pandas/dagster_duckdb_pandas_tests/test_type_handler.py
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
we are using an additional type handler for the duckdb io_manager which uses views to link data into the duckdb database to avaoid copying the data. unfrotunately the current io_manager always tries to delete the data in a table even when it's a view. this change ignores exceptions due to views.
How I Tested These Changes
i made this changes locally in our dagster instance and it work's with our custom type handler.
Changelog
dagster-duckdb: gracefully handle table cleanup exceptions on views