feat(ingest/postgres): support extracting metadata from all databases in single recipe #7581
Conversation
mayurinehate
changed the title
github-actions
bot
added
the
ingestion
| @@ -101,13 +105,14 @@ class PostgresConfig(BasicSQLAlchemyConfig): | |||
| default=False, description="Include table lineage for views" | |||
| ) | |||
|
|
|||
| def get_identifier(self: BasicSQLAlchemyConfig, schema: str, table: str) -> str: | |||
There was a problem hiding this comment.
Not required as we override get_identifier within PostgresSource
mayurinehate
force-pushed
the
postgres_support_all_databases
branch
from
27a5d6f
to
b284ed3
Compare
...ta-ingestion/tests/integration/postgres/postgres_to_file_with_db_estimate_row_count copy.yml
Outdated
Show resolved
Hide resolved
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
The logic to construct urn remains same in this PR. Also, AFAIK, running postgres source without |
| logger.debug(f"sql_alchemy_url={url}") | ||
| engine = create_engine(url, **self.config.options) | ||
| with engine.connect() as conn: | ||
| if self.config.database and self.config.database != "": |
There was a problem hiding this comment.
This is redundant -- "" is falsy
| inspector = inspect( | ||
| create_engine(url, **self.config.options).connect() | ||
| ) | ||
| yield inspector |
There was a problem hiding this comment.
Do we want to keep conn open as we run ingestion on each inspector? If we store databases in memory then we can move the iteration outside the with block to close conn. I don't think it really matters either way though
More importantly, are these inspectors getting closed?
| url = self.config.get_sql_alchemy_url() | ||
| engine = create_engine(url, **self.config.options) | ||
| def _get_view_lineage_elements( | ||
| self, inspector: Inspector |
There was a problem hiding this comment.
Just curious, how come we're using Inspectors over a straight conn (or whatever the result of .connect() is?
There was a problem hiding this comment.
Using connection directly does make sense indeed. especially for this piece _get_view_lineage_elements, as it does not require inspector. Inspector interface is used in sql_common to get metadata of tables, etc and probably that's why needs to be used here as well.
… in single recipe (#7581) Co-authored-by: Tamas Nemeth <treff7es@gmail.com>
Checklist