Skip to content
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

Implement new Google Search 360 operators #79

Closed

Conversation

molcay
Copy link
Collaborator

@molcay molcay commented Jul 24, 2024

Implement new Google Search 360 operators
Remove the decommissioned operators

We will just wait for the platform account to run the system test

Here is a bit of an explanation for clearer understanding;

The previous Search Ads 360 Reporting API (which is currently in use in google-provider) was already decommissioned on June 30, 2024 (see deatils). All new reporting development should use the new Search Ads 360 Reporting API.
Currently, the Reporting operators, sensors and hooks are failing due to the decommission. The new API is not a replacement for the old one, it has a different approach and APIs. Therefore, we needed to implement new operators and remove the old ones. This PR does NOT break any working DAG, because it replaces not working functionality with the new one.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@molcay molcay self-assigned this Jul 24, 2024
@molcay molcay force-pushed the feat/new-google-search-ads-reporting-api-operators branch from 28cd5d3 to 7d3adba Compare July 25, 2024 09:01
@molcay molcay changed the title Implement new Google Search 360 operators [WIP] Implement new Google Search 360 operators Jul 25, 2024
@molcay molcay changed the title [WIP] Implement new Google Search 360 operators Implement new Google Search 360 operators Jul 25, 2024
Copy link
Collaborator

@moiseenkov moiseenkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. But could you please add a more explicit explanation that new operators replace the old, and we are deleting them because the old API reached the end of life and doesn't work anymore. I believe it would speed up a code review in the community and avoid unnecessary questions about breaking changes. We are not breaking anything. We are replacing broken operators with new working operators.

@molcay molcay force-pushed the feat/new-google-search-ads-reporting-api-operators branch from 7d3adba to 83d7595 Compare July 29, 2024 08:25
@molcay
Copy link
Collaborator Author

molcay commented Jul 29, 2024

Hi @moiseenkov,
I changed the description of the ticket and included a paragraph about why we are doing this change. Can you have a look?

@molcay molcay force-pushed the feat/new-google-search-ads-reporting-api-operators branch from 83d7595 to 45021c6 Compare July 29, 2024 08:57
@moiseenkov
Copy link
Collaborator

Hi @moiseenkov, I changed the description of the ticket and included a paragraph about why we are doing this change. Can you have a look?

Thanks for a very detailed description. I'd only correct the last sentence:

This PR does NOT break any working DAG, because it replaces not working functionality with the new one.

@MaksYermak
Copy link
Collaborator

@molcay could you please recreate this PR or rebase it, based on VladaZakharova/main branch?

@molcay molcay force-pushed the feat/new-google-search-ads-reporting-api-operators branch 3 times, most recently from 8e84b3a to b010198 Compare September 9, 2024 08:28
@molcay molcay force-pushed the feat/new-google-search-ads-reporting-api-operators branch 2 times, most recently from f30d7bf to f146944 Compare September 10, 2024 08:06
Copy link
Collaborator

@MaksYermak MaksYermak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@molcay LGTM

molcay and others added 3 commits September 16, 2024 12:17
Remove the decommissioned operators

# Conflicts:
#	tests/providers/google/marketing_platform/operators/test_search_ads.py
The twisted/incremental#106 issue has
been addressed in 24.7.2 so we are removing the limit - just in
case we also exclude the buggy versions, even if they are yanked.
Remove the decommissioned operators
@molcay molcay force-pushed the feat/new-google-search-ads-reporting-api-operators branch from f146944 to e3cd2e4 Compare September 16, 2024 12:18
@molcay molcay closed this Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants