jacobhjkim commented on a change in pull request #15266:
URL: https://github.com/apache/airflow/pull/15266#discussion_r619671516
##########
File path: airflow/providers/google/ads/transfers/ads_to_gcs.py
##########
@@ -92,6 +94,7 @@ def __init__(
page_size: int = 10000,
gzip: bool = False,
impersonation_chain: Optional[Union[str, Sequence[str]]] = None,
+ api_version: str = "v5",
Review comment:
Since passing in `None` is not the same as not passing in any parameter,
I will have to change `GoogleAdsHook` to something like this. Does this look ok?
```python
def __init__(
self,
api_version: str,
gcp_conn_id: str = "google_cloud_default",
google_ads_conn_id: str = "google_ads_default",
) -> None:
super().__init__()
self.api_version = api_version if api_version else "v5"
...
```
This doesn't feel Pythonic, but it does allow us to only change one place in
the future.
I wanted to see how other provider hooks deal with `api_version`
1.
[ComputeEngineHook](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/hooks/compute.py#L52)
takes `api_version` as an argument, but
[ComputeEngineSSHHook](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/hooks/compute_ssh.py#L144)
which calls ComputeEngineHook doesn't pass in `api_version`.
2.
[CloudSQLHook](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/hooks/cloud_sql.py#L96)
takes `api_version` but has no default value. Instead
[CloudSQLCreateInstanceOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/operators/cloud_sql.py#L320)
has a default value assigned.
It seems like there is no set method for this specific issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
