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

refactor: Optimise metadata discovery for large databases #528

Merged
merged 2 commits into from
Nov 5, 2024
Merged

refactor: Optimise metadata discovery for large databases #528

merged 2 commits into from
Nov 5, 2024

Conversation

jamesmeneghello
Copy link
Contributor

@jamesmeneghello jamesmeneghello commented Nov 4, 2024 •
edited by edgarrmondragon
Loading

Overrides the SDK functions to instead use the get_multi_* functions from SQLAlchemy Inspector. On our database of ~120 tables, this reduces the discovery runtime from 10-12 minutes to about 30 seconds.

edgarrmondragon
edgarrmondragon reviewed Nov 4, 2024
View reviewed changes
Copy link
Member

@edgarrmondragon edgarrmondragon left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @jamesmeneghello! I left some comments.

tap_postgres/client.py Outdated Show resolved Hide resolved
tap_postgres/client.py Outdated Show resolved Hide resolved
tap_postgres/client.py Show resolved Hide resolved
@edgarrmondragon edgarrmondragon changed the title Optimise metadata discovery for large databases refactor: Optimise metadata discovery for large databases Nov 4, 2024
@edgarrmondragon edgarrmondragon self-assigned this Nov 4, 2024
@edgarrmondragon edgarrmondragon added the enhancement New feature or request label Nov 4, 2024
@jamesmeneghello
Copy link
Contributor Author

Fixed those and the issues from the CI run.

edgarrmondragon
edgarrmondragon approved these changes Nov 5, 2024
View reviewed changes
Copy link
Member

@edgarrmondragon edgarrmondragon left a comment

Choose a reason for hiding this comment

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

Thanks @jamesmeneghello!

@edgarrmondragon edgarrmondragon merged commit 9bb40d1 into MeltanoLabs:main Nov 5, 2024
12 checks passed
@edgarrmondragon edgarrmondragon mentioned this pull request Nov 5, 2024
Open
edgarrmondragon added a commit that referenced this pull request Nov 6, 2024
@edgarrmondragon
revert: Optimise metadata discovery for large databases
800a133 
Related:

- Reverts #528
- Closes #535

This reverts commit 9bb40d1.
@edgarrmondragon edgarrmondragon mentioned this pull request Nov 6, 2024
Merged
edgarrmondragon added a commit that referenced this pull request Nov 7, 2024
@edgarrmondragon
revert: Optimise metadata discovery for large databases (#536)
c436267 
Related:

- Reverts #528
- #535

This reverts commit 9bb40d1.
@edgarrmondragon
Copy link
Member

edgarrmondragon commented Nov 7, 2024 •
edited
Loading

This seems to have broken at least stream maps (#535), so I reverted it in #536 until I can investigate and come up with a patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edgarrmondragon edgarrmondragon edgarrmondragon approved these changes

Assignees

@edgarrmondragon edgarrmondragon

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Discovery Performance
2 participants
@jamesmeneghello @edgarrmondragon