Skip to content

Conversation

@darynaishchenko
Copy link
Collaborator

@darynaishchenko darynaishchenko commented Dec 14, 2023

What

resolved: https://github.com/airbytehq/oncall/issues/3466
401 Request is missing required authentication credential

resolved: #19455

How

Added config error in case of 401 error. Updated ad_groups schema.

@darynaishchenko darynaishchenko self-assigned this Dec 14, 2023
@vercel
Copy link

vercel bot commented Dec 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Jan 9, 2024 3:54pm
@github-actions
Copy link
Contributor

github-actions bot commented Dec 14, 2023

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

  • PR name follows PR naming conventions
  • Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
  • Secrets in the connector's spec are annotated with airbyte_secret
  • All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
  • Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
  • If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

@octavia-squidington-iv octavia-squidington-iv requested a review from a team December 18, 2023 12:26
@darynaishchenko darynaishchenko changed the title 🐛 Source Google Ads: added handling for 401 error while parsing response 🐛 Source Google Ads: added handling for 401 error while parsing response. added metrics.cost_micros to ad_groups stream Dec 18, 2023
@github-actions
Copy link
Contributor

Warning

🚨 Connector code freeze is in effect until 2024-01-02. This PR is changing connector code. Please contact the current OC engineers if you want to merge this change to master.

try:
yield self.google_ads_client.parse_single_result(self.get_json_schema(), result)
except Unauthenticated:
raise AirbyteTracedException(
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to incorporate this error handling into the traced_exception function located in utils.py? This would centralize error handling, making it more manageable and consistent across the connector.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated with traced_exception

@darynaishchenko darynaishchenko requested a review from tolik0 January 8, 2024 15:35
@darynaishchenko darynaishchenko merged commit 0764678 into master Jan 9, 2024
@darynaishchenko darynaishchenko deleted the daryna/source-google-ads/oc-auth-error branch January 9, 2024 16:35
@tolik0
Copy link
Contributor

tolik0 commented Jan 11, 2024

@darynaishchenko, the update to add a new field to the ad_group stream is leading to errors for users with the connector set up using a manager account. This is because metrics are not accessible for manager accounts. Here's a snippet from the log indicating the issue:

2024-01-11 18:05:50 source > Marking stream ad_group as STARTED 2024-01-11 18:05:50 source > Syncing stream: ad_group 2024-01-11 18:05:51 source > Request made: ClientCustomerId: 9016006472, Host: googleads.googleapis.com, Method: /google.ads.googleads.v15.services.GoogleAdsService/Search, RequestId: 07AX5lxXUaVGq_jhuKSV3w, IsFault: True, FaultMessage: Metrics cannot be requested for a manager account. To retrieve metrics, issue separate requests against each client account under the manager account. 2024-01-11 18:05:51 source > Finished syncing ad_group 2024-01-11 18:05:51 source > SourceGoogleAds runtimes: Syncing stream ad_group 0:00:00.317514 2024-01-11 18:05:51 source > (<_InactiveRpcError of RPC that terminated with: status = StatusCode.INVALID_ARGUMENT details = "Request contains an invalid argument." debug_error_string = "UNKNOWN:Error received from peer ipv4:142.251.208.170:443 {created_time:"2024-01-11T18:05:51.006095582+00:00", grpc_status:3, grpc_message:"Request contains an invalid argument."}" >, <_InactiveRpcError of RPC that terminated with: status = StatusCode.INVALID_ARGUMENT details = "Request contains an invalid argument." debug_error_string = "UNKNOWN:Error received from peer ipv4:142.251.208.170:443 {created_time:"2024-01-11T18:05:51.006095582+00:00", grpc_status:3, grpc_message:"Request contains an invalid argument."}" >, errors { error_code { query_error: REQUESTED_METRICS_FOR_MANAGER 

Since blocking this stream for manager accounts impacts other customers' data, I suggest we revert this change.

jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024
jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/google-ads

5 participants