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

chore: uncomment SnowflakeTargetSchemaUpdates test #45

Merged
merged 1 commit into from
Jun 8, 2023

Conversation

pnadolny13
Copy link
Contributor

Closes #43

It turns out that this test had been fixed by other changes. I suspect it was part of #39. Previously I had implemented a way to cache table properties but I suspect that the cache was not being invalidated when the schema updated so it was attempting to recreate an existing column since it didnt find it in the cache.

@kgpayne
Copy link
Collaborator

kgpayne commented Jun 8, 2023

@pnadolny13 great! We have caching coming up-stream in the SDK, which is why I dropped it in the refactor 😅

@pnadolny13
Copy link
Contributor Author

@kgpayne I think the main bottle neck was in requesting column metadata for every column instead of caching it once for the table, I logged it in #29 though.

I also wonder in meltano/sdk#1745 if it would be subject to the same bug I described here where schema changes within a single sync don't invalidate the cache 🤔 .

@pnadolny13 pnadolny13 merged commit 9a79039 into main Jun 8, 2023
@pnadolny13 pnadolny13 deleted the fix_test_target_schema_update branch June 8, 2023 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

bug: default test TargetSchemaUpdates fails column 'A3' already exists
2 participants