-
Notifications
You must be signed in to change notification settings - Fork 130
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
Metadata.reflect() fails if user does not have access to all datasets/tables in project. #838
Comments
Please note, I can also confirm that the dataset redacted in the error message does exist, I just dont have permissions to it. This dataset is not described in my "only" table list, so I should not need to have permissions to this dataset. |
Based on my understanding of the code in sqlalchemy, it is sqlalchemy that should be doing the filtering to determine which tables to request for reflection. We should only be getting a filtered list of tables that they request. In the section above, there are several snippets of code that look like this one that basically parse the current list of all tables and then if
It is not clear how our code gets an unfiltered list of tables but I recommend that you start by looking at the sqlalchemy code to ensure that their logic is correct and whether it really is sending out a filtered list. Gonna move this down to a P3, unless we get feedback that sqlalchemy is performing as expected. |
In the absence of additional evidence that this is truly an issue with |
You can specify # MetaData.reflect(
# ...
# insert this to fix:
# available = util.OrderedSet({v.split('.', 1)[-1] for v in available})
if schema is not None:
available_w_schema: util.OrderedSet[str] = util.OrderedSet(
[f"{schema}.{name}" for name in available]
)
else:
available_w_schema = available But table What is bad on SqlAlchemy side - it forces dialect to load all table names in that schema. |
Yeah, I tried playing around with the use of schema about a month ago and I could not get it working with a |
The BigQuery dialect is actually violating the interface here I think - all of the other dialects return only the table name from
This part is a slightly separate from the OP though, so I'll open another issue for it. This also wreaks havoc when using |
The issue is that the python-bigquery-sqlalchemy/sqlalchemy_bigquery/base.py Lines 1103 to 1112 in 155e5b0
Those methods are only supposed to operate on a single schema at a time iiuc, so this should be fixed in #1089, but the OP would need to still call |
Thanks for stopping by to let us know something could be better!
PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.
Please run down the following list and make sure you've tried the usual "quick fixes":
If you are still having issues, please be sure to include as much information as possible:
Filed this issue on main SQLalchemy Git project, but they think this is a BigQuery driver issue:
sqlalchemy/sqlalchemy#9319
Environment details
sqlalchemy-bigquery
version: 1.5.0Steps to reproduce
Metadata.reflect function tries to get all table names in engine before using the "only" table list. This can cause issues if a user does not have read permissions on all tables/datasets under engine, and results in a permission denied error.
Check for all table names: https://github.com/sqlalchemy/sqlalchemy/blob/main/lib/sqlalchemy/sql/schema.py#L5487
Check for "only" tables after: https://github.com/sqlalchemy/sqlalchemy/blob/main/lib/sqlalchemy/sql/schema.py#L5507
Desired behavior is to only check on tables explicitly listed with "only" argument.
Code example
Simple example similar to my problem. Engine is configured to a certain BQ project. My "only" table list has multiple tables coming from different datasets withing the project. I also do not have permission to all tables/datasets within the project.
Stack trace
Partially redacted stack and error message.
Making sure to follow these steps will guarantee the quickest resolution possible.
Thanks!
The text was updated successfully, but these errors were encountered: