-
Notifications
You must be signed in to change notification settings - Fork 25
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
865 the hook needs to contain similar kwargs as the google providers #866
865 the hook needs to contain similar kwargs as the google providers #866
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
rebase once the PR gets merged #867 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please add this additional param to example DAG as well?
FYI: Look like google async lib have some issue with non us location apache/airflow#29307 |
One of the errors, currently, is a permission issue. It would make sense that it's a region issue, but I haven't verified for sure, since it's not my service-account. We should include this and just update the docs to reflect a possible issue with non-US locations. |
…-as-the-google-providers
Codecov ReportBase: 98.61% // Head: 98.61% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #866 +/- ##
=======================================
Coverage 98.61% 98.61%
=======================================
Files 89 89
Lines 4987 4987
=======================================
Hits 4918 4918
Misses 69 69
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
We have an example here using location param https://github.com/astronomer/astronomer-providers/blob/main/astronomer/providers/google/cloud/example_dags/example_bigquery_impersonation_chain.py#L92 do you mean something else? |
Can we fix the failing tests as well? If you need any help with tests do let us know. |
I've gone ahead and removed the |
Can you rebase the branch? |
…-as-the-google-providers
I have added the
location
parameter in the hook. I do think this needs to get upgraded to integrate theuse_legacy_sql
parameter, as well, but that change may not be so simple given it could change the requirements.txt.