feat(ingestion/trino): Add sibling support in ingestion #9853
Conversation
github-actions
bot
added
ingestion
| class TrinoConfig(BasicSQLAlchemyConfig): | ||
| # defaults | ||
| scheme: str = Field(default="trino", description="", hidden_from_docs=True) | ||
|
|
||
| catalog_to_connector_details: Dict[str, ConnectorDetail] = Field( |
There was a problem hiding this comment.
let's make sure we're using the terms catalog and connector in a way that's consistent with https://trino.io/docs/current/overview/concepts.html#data-sources
The docs for this are also pretty unclear - terms like "three tier connector" are not commonplace
There was a problem hiding this comment.
Let me know if its still unclear
| catalog_name, ConnectorDetail() | ||
| ) | ||
|
|
||
| if connector_platform_details: |
There was a problem hiding this comment.
why can't we just always try to generate this relationship?
There was a problem hiding this comment.
Because there can be some trino connector platform which datahub doesn't support yet.
Eg: https://trino.io/docs/current/connector/hudi.html
| """ | ||
| ).strip() | ||
| res = connection.execute(sql.text(query)) | ||
| catalog_connector_dict = {row.catalog_name: row.connector_name for row in res} |
There was a problem hiding this comment.
split this up into two methods - one to generate catalog_connector_dict, and the second doing the lookup
the lru_cache annotation should be on the first one
| platform_instance=connector_details.platform_instance, | ||
| env=connector_details.env, | ||
| ) | ||
| elif connector_details.connector_database: # else connector is three tier |
There was a problem hiding this comment.
add an else clause that reports a warning that the connector_database is missing
| ) | ||
| connector_platform_name = KNOWN_CONNECTOR_PLATFORM_MAPPING.get( | ||
| connector_details.connector_platform | ||
| if connector_details.connector_platform |
There was a problem hiding this comment.
connector_details.connector_platform or connector_name
asikowitz
removed
the
community-contribution
Checklist