-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Rename CloudBaseHook to GoogleBaseHook and move it to google.common #8011
Rename CloudBaseHook to GoogleBaseHook and move it to google.common #8011
Conversation
d47bfa2
to
37de5dc
Compare
37de5dc
to
b907311
Compare
@@ -23,10 +23,10 @@ | |||
|
|||
from googleapiclient.discovery import Resource, build | |||
|
|||
from airflow.providers.google.cloud.hooks.base import CloudBaseHook | |||
from airflow.providers.google.common.hooks.base import GoogleBaseHook |
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.
Should we move this hook to common package?
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.
I think so, good catch! @potiuk WDYT?
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.
Thanks for doing this.
Hi. Travis is sad. This is a blocker for me because I would like to take care of the latest improvements. in the package/class name. |
@@ -29,10 +29,10 @@ | |||
ImageObjectDetectionModelDeploymentMetadata, InputConfig, Model, Operation, PredictResponse, TableSpec, | |||
) | |||
|
|||
from airflow.providers.google.cloud.hooks.base import CloudBaseHook | |||
from airflow.providers.google.common.hooks.base import GoogleBaseHook |
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.
from airflow.providers.google.common.hooks.base import GoogleBaseHook | |
from airflow.providers.google.common.hooks.base_google import GoogleBaseHook |
We agreed to such a convention in the case of AWS. In the case of Google, we should also use it. I will add a commit to this branch, which corrects it. In next step, I will start new PR starting from this branch, which will correct other errors.
568d914
to
a4f6286
Compare
Issue link: WILL BE INSERTED BY boring-cyborg
Make sure to mark the boxes below before creating PR: [x]
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.