fix: Fix metadata reflection without a default dataset#1089
fix: Fix metadata reflection without a default dataset#1089JacobHayes wants to merge 3 commits intogoogleapis:mainfrom
Conversation
c35e20e to
7d72652
Compare
|
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
969685f to
4a006bf
Compare
|
Hi, any chance I could get the tests approved to run on the PR? I've been trying to setup the system tests locally, but they're a bit onerous (understandably). |
|
Sorry for the delay, just started running the tests! |
no worries, thank you! |
64ab97f to
75a933a
Compare
|
@lingchin if this is a breaking change as Jacob indicates, then we need to have some sort of deprecation warning that the break is coming and get this into the schedule.
|
Fixes #838 and fixes #1088. 🦕
The
get_table_namesandget_view_namesmethods are supposed to return the bare names (no{schema}.prefix) of the resources for a single schema/dataset (whereschema=Noneis the "default schema", not "all schemas"). However, theBigQueryDialectimplementation returns:schema: if the connection has a default dataset{schema}.prefixed names for one schema: if the connection doesn't have a default dataset and theschemaarg is a string{schema}.prefixed names for all schemas: if the connection doesn't have a default dataset and theschemaarg isNoneThe bolded parts cause issues outlined in #1088. This PR fixes the
get_table_namesandget_view_namesimplementations to return bare names as SQLAlchemy expects.This is a breaking change for a subset of users:
Metadata.reflect()without aschemaargument were probably expecting to reflect all datasets, but they would now not reflect anything.public) - so without a default set on the connection string, we don't have anything to search and return an empty list.This has been working for me locally with
alembic+ no default dataset (the models specify the schema) +include_schemas=True(required for cross-schema stuff).