You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/04/12 22:28:14 UTC
[GitHub] [airflow] anthonyjgraff opened a new pull request, #22965: Upgrade to support Google Ads v10
anthonyjgraff opened a new pull request, #22965:
URL: https://github.com/apache/airflow/pull/22965
Closes: https://github.com/apache/airflow/issues/22111
### Description
This PR updates the google-ads dependency to v15.1.1, which adds support for v10 of the Google Ads API.
The latest version of the Google Ads API supported by the current library is v8, which will be sunsetted on 2022-05-11 (https://developers.google.com/google-ads/api/docs/sunset-dates).
Previously, Airflow was limited to v14.0.0 of the library as more recent versions required v2.x of google-api-core. However, the google-ads library re-added support for v1.x of google-api-core in v15.1.1
### Testing
This is my first Airflow PR - I tried to run the full Breeze test suite, but struggled as the new requirements conflict with the constraints files - any advice would be welcome!
However, I did build an image & run a few DAGs / operators that I run regularly, getting the expected result
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1115939714
Yes. It's merged. You will have to wait unit Google Provider gets released (which is going to happen in the next few days as we are preparing for a new wave of those.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1099979608
Likely transient failure. You can always "rerun failed jobs" if it happens.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1097292325
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, itโs a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better ๐.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #22965:
URL: https://github.com/apache/airflow/pull/22965#discussion_r850244964
##########
setup.cfg:
##########
@@ -91,7 +91,7 @@ install_requires =
# Cattrs upgrades were known to break lineage https://github.com/apache/airflow/issues/16172
# TODO: Cattrs is now at 3.8 version so we should attempt to upgrade cattrs soon.
cattrs~=1.1, !=1.7.*
- colorlog>=4.0.2
+ colorlog>=4.0.2, <5.0
Review Comment:
Separate PR is way better. Just comment in this one as we have a rule thall all upper-bounded limitations have to have comment why and what is needed to lift it.
https://github.com/apache/airflow/blob/main/README.md#approach-for-dependencies-for-airflow-core
> Whenever we upper-bound such a dependency, we should always comment why we are doing it - i.e. we should have a good reason why dependency is upper-bound. And we should also mention what is the condition to remove the binding.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] k1my0 commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
k1my0 commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1115786107
How can I use it? Is this PR merged normally?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #22965:
URL: https://github.com/apache/airflow/pull/22965
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1119942565
> Sounds great. Just as a heads up, V8 of the API will stop working on Wednesday 11th - definitely makes sense to group the Google Provider changes together, but we'll be a bit stuck if a new version of the provider hasn't been released by then!
I am going to send it to voting during the weekend most likely. It will take min 72 hours, But if bugs are found and we cancel the RC it might take multiple of those 72 hrs. So I think if your business depends on it , you should work on a contingency plan. One of the options you have is taking the unreleased code of the providers and copy it to your dags folder and use it from there.
But I am not sure also if everyone is able to upgrade after that - the new providers will not work for those on 2.0 for example, but this is something that is on Google Ads team to drop the APIs, this is not something we can do about, if the ads team make the decision to switch off the APIs on Wed, then for sure accepted the risk that some users will not be able to upgrade some of the clients - and they are the only ones you can appeal to if you badly need it to continue working.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jiamo commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
jiamo commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1125008088
I found code in google ads v10
```
def search(
self,
request: Union[google_ads_service.SearchGoogleAdsRequest, dict] = None,
*,
customer_id: str = None,
query: str = None,
retry: OptionalRetry = gapic_v1.method.DEFAULT,
timeout: float = None,
metadata: Sequence[Tuple[str, str]] = (),
) -> pagers.SearchPager:
r"""Returns all rows that match the search query.
List of thrown errors: `AuthenticationError <>`__
`AuthorizationError <>`__ `ChangeEventError <>`__
`ChangeStatusError <>`__ `ClickViewError <>`__
`HeaderError <>`__ `InternalError <>`__ `QueryError <>`__
`QuotaError <>`__ `RequestError <>`__
Args:
request (Union[google.ads.googleads.v10.services.types.SearchGoogleAdsRequest, dict]):
The request object. Request message for
[GoogleAdsService.Search][google.ads.googleads.v10.services.GoogleAdsService.Search].
customer_id (str):
Required. The ID of the customer
being queried.
```
So in `airflow/providers/google/ads/hooks/ads.py` we need add
change
```
iterator = service.search(request=request)
```
to
```
iterator = service.search(request=request, customer_id=client_id)
```
But the `request` already added the `customer_id`. I am not sure for this.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] anthonyjgraff commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
anthonyjgraff commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1099943165
Any idea what would have caused the above failure? It doesn't look like it's related to anything changed in this PR.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] anthonyjgraff commented on a diff in pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
anthonyjgraff commented on code in PR #22965:
URL: https://github.com/apache/airflow/pull/22965#discussion_r850128387
##########
setup.cfg:
##########
@@ -91,7 +91,7 @@ install_requires =
# Cattrs upgrades were known to break lineage https://github.com/apache/airflow/issues/16172
# TODO: Cattrs is now at 3.8 version so we should attempt to upgrade cattrs soon.
cattrs~=1.1, !=1.7.*
- colorlog>=4.0.2
+ colorlog>=4.0.2, <5.0
Review Comment:
It felt fixable when I first spotted it yesterday - the issues all seemed to be related to `CustomTTYColoredFormatter` but it didn't feel right to make changes to Airflow logging as part of this PR, as it's totally unrelated to Google Ads
It was actually mypy that threw up errors - the `TTYColoredFormatter` class we inherit from was merged into `ColoredFormatter`, and as a result, a few functions have changed names / signatures.
Can introduce a comment as part of this PR, and can look to upgrade it in a separate PR
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SarbjitGahra commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
SarbjitGahra commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1119926688
> Sounds great. Just as a heads up, V8 of the API will stop working on Wednesday 11th - definitely makes sense to group the Google Provider changes together, but we'll be a bit stuck if a new version of the provider hasn't been released by then!
Did you find any solution for this ? I am running into same issue. Unable to update to the latest version because of google provider requirement range being <14.0.1
Any help is appreciated?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #22965:
URL: https://github.com/apache/airflow/pull/22965#discussion_r850244964
##########
setup.cfg:
##########
@@ -91,7 +91,7 @@ install_requires =
# Cattrs upgrades were known to break lineage https://github.com/apache/airflow/issues/16172
# TODO: Cattrs is now at 3.8 version so we should attempt to upgrade cattrs soon.
cattrs~=1.1, !=1.7.*
- colorlog>=4.0.2
+ colorlog>=4.0.2, <5.0
Review Comment:
Separate PR is way better. Just comment in this one as we have a rule thall all upper-bounded limitations have to have comment why and what is needed to lift it.
> [](https://github.com/apache/airflow/blob/main/README.md#approach-for-dependencies-for-airflow-core)Whenever we upper-bound such a dependency, we should always comment why we are doing it - i.e. we should have a good reason why dependency is upper-bound. And we should also mention what is the condition to remove the binding.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1100021464
Awesome work, congrats on your first merged pull request!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jiamo commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
jiamo commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1125826923
Re apply helm file with the latest 7.0.0rc works fine. So the problem only happened at just pip install 7.0.0rc.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jiamo commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
jiamo commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1124681967
Hi. after install `pip install apache-airflow-providers-google==7.0.0rc1` and change `v8` to `v10`
```
service = GoogleAdsHook(
gcp_conn_id=self.gcp_conn_id,
google_ads_conn_id=self.google_ads_conn_id,
api_version="v8") # here change
rows = service.search(client_ids=self.client_ids,
query=self.query, page_size=self.page_size)
```
I got error :
```
TypeError: Invalid constructor input for SearchGoogleAdsRequest: customer_id:
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] anthonyjgraff commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
anthonyjgraff commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1118383629
Sounds great. Just as a heads up, V8 of the API will stop working on Wednesday 11th - definitely makes sense to group the Google Provider changes together, but we'll be a bit stuck if a new version of the provider hasn't been released by then!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #22965:
URL: https://github.com/apache/airflow/pull/22965#discussion_r850003787
##########
setup.cfg:
##########
@@ -91,7 +91,7 @@ install_requires =
# Cattrs upgrades were known to break lineage https://github.com/apache/airflow/issues/16172
# TODO: Cattrs is now at 3.8 version so we should attempt to upgrade cattrs soon.
cattrs~=1.1, !=1.7.*
- colorlog>=4.0.2
+ colorlog>=4.0.2, <5.0
Review Comment:
We should describae why we limited it - which tests failed - ideally what were the proble and what we need to fix it.
Or maybe we can fix it ? I doubt colorlog has some "really big" changes.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1099263645
Cool. I removed the empty requirements.txt added there by mistake :)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] anthonyjgraff commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
anthonyjgraff commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1098488636
Looks like the build is failing as `colorlog` is updating from v4.8 to v6.x, which has some backwards incompatible changes
Will pin `colorlog` to < 5 in this PR
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jiamo commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
jiamo commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1124805004
The total trace
```
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/task/task_runner/standard_task_runner.py", line 85, in _start_by_fork
args.func(args, dag=self.dag)
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/cli/cli_parser.py", line 48, in command
return func(*args, **kwargs)
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/utils/cli.py", line 92, in wrapper
return f(*args, **kwargs)
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/cli/commands/task_command.py", line 298, in task_run
_run_task_by_selected_method(args, dag, ti)
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/cli/commands/task_command.py", line 107, in _run_task_by_selected_method
_run_raw_task(args, ti)
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/cli/commands/task_command.py", line 180, in _run_raw_task
ti._run_raw_task(
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/utils/session.py", line 70, in wrapper
return func(*args, session=session, **kwargs)
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1334, in _run_raw_task
self._execute_task_with_callbacks(context)
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1460, in _execute_task_with_callbacks
result = self._execute_task(context, self.task)
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1516, in _execute_task
result = execute_callable(context=context)
File "/opt/airflow/dags/repo/plugins/ads_to_postgres.py", line 55, in execute
rows = service.search(client_ids=self.client_ids,
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/google/ads/hooks/ads.py", line 113, in search
"""
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/google/ads/hooks/ads.py", line 234, in _search
File "/home/airflow/.local/lib/python3.8/site-packages/google/ads/googleads/v10/services/services/google_ads_service/client.py", line 3306, in search
request = google_ads_service.SearchGoogleAdsRequest(request)
File "/home/airflow/.local/lib/python3.8/site-packages/proto/message.py", line 486, in __init__
# Just use the above logic on mapping's underlying pb.
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jiamo commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
jiamo commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1124803857
self.client_ids was query from db . it is an array. Same code works for `v8`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1124807105
BTW. The google provider (including ads10 support) is being voted right now. I encourage everyone to test it and report status at #23659
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] anthonyjgraff commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
anthonyjgraff commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1125496594
@jiamo
Have spent some time looking through your traceback & having a read of the code
You have this line in your traceback:
```
File "/home/airflow/.local/lib/python3.8/site-packages/google/ads/googleads/v10/services/services/google_ads_service/client.py", line 3306, in search
```
This line is only hit when:
- We have v10 of `GoogleAdsServiceClient` (returned by `GoogleAdsHook._get_service`)
- The request object isn't a v10 `SearchGoogleAdsRequest` (see line 3305 above)
As the search function creates a `SearchGoogleAdsRequest` object from the `GoogleAdsClient` returned by `GoogleAdsHook._get_client`, I'm guessing this means you've somehow got a v8 / v9 version of the client.
I haven't been able to create a scenario where the service is newer than the request object, but I did manage to create the reverse scenario where a v10 request object was sent to a v9 service and got the following, (which gave a pretty similar traceback)!
```
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/google/ads/hooks/ads.py", line 114, in search
data_proto_plus = self._search(client_ids, query, page_size, **kwargs)
File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/google/ads/hooks/ads.py", line 235, in _search
iterator = service.search(request=request)
File "/home/airflow/.local/lib/python3.8/site-packages/google/ads/googleads/v9/services/services/google_ads_service/client.py", line 3163, in search
request = google_ads_service.SearchGoogleAdsRequest(request)
File "/home/airflow/.local/lib/python3.8/site-packages/proto/message.py", line 496, in __init__
raise TypeError(
TypeError: Invalid constructor input for SearchGoogleAdsRequest: customer_id: "2983456428"
query: "\n SELECT\n customer_client.id,\n customer_client.status\n FROM customer_client\n WHERE customer_client.level <= 1\n "
page_size: 10000
```
In my case, I suspect the bug was caused because we don't pass `self.api_version` through when calling `GoogleAdsClient.load_from_dict` in the `GoogleAdsHook` (meaning the client defaults to the latest version, whilst the service correctly uses v9).
From what I can tell, this behaviour long pre-dates this PR (so not sure how big of an issue it is with regards to https://github.com/apache/airflow/issues/23659), but can investigate further tomorrow.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jiamo commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
jiamo commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1125601432
It is strange I don't upgrade airflow and login in the worker pod.
same logic code . just run `python` and paste
```
from operator import attrgetter
from tempfile import NamedTemporaryFile
from typing import Dict, List, Optional, Sequence
from airflow.models import BaseOperator
from airflow.plugins_manager import AirflowPlugin
from airflow.providers.google.ads.hooks.ads import GoogleAdsHook
query = """
SELECT customer.id,
customer.descriptive_name,
campaign.id, campaign.name, ad_group.id, ad_group.name,
geographic_view.country_criterion_id,
segments.date,
metrics.cost_micros, metrics.impressions, metrics.clicks, metrics.conversions, metrics.ctr
FROM geographic_view
WHERE segments.date=\'2022-05-11\'
"""
service = GoogleAdsHook(gcp_conn_id='onion-16040', google_ads_conn_id='google_ads_default', api_version="v10")
rows = service.search(client_ids=["xxxxxxxxxxx"], query=query, page_size=10000)
```
It works fine.
So `GoogleAdsHook` and `search` was fine. So I suspect just execute `pip install apache-airflow-providers-google==7.0.0rc` don't have a complete upgrade process.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] anthonyjgraff commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
anthonyjgraff commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1124788187
@jiamo `customer_id` comes from the value passed to `client_ids` - do you know what value you're using for `self.client_ids`?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1119945340
Just wanted you to give heads up @SarbjitGahra @anthonyjgraff that if your business continuity depends on the ads, this is not something our decisions on releasing or not releasing the provider are based. It's your thing to make sure you have contingency plan.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] k1my0 commented on pull request #22965: Upgrade to support Google Ads v10
Posted by GitBox <gi...@apache.org>.
k1my0 commented on PR #22965:
URL: https://github.com/apache/airflow/pull/22965#issuecomment-1116882343
Thanks!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org