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

Add retry logic #22

Merged
merged 2 commits into from
Mar 1, 2022
Merged

Add retry logic #22

merged 2 commits into from
Mar 1, 2022

Conversation

dsprayberry
Copy link
Contributor

Description of change

Adds error retry logic and giveup. Logs failed request on giveup / after retry.

QA steps

  • automated tests passing
  • manual qa steps passing (list below)
  • wrote and ran unit tests test_requests.py

Risks

  • Minimal. Could increase the length of extractions when errors are encountered.

Rollback steps

  • revert this branch

@dsprayberry dsprayberry requested a review from luandy64 March 1, 2022 19:05
@dsprayberry dsprayberry merged commit d8f30cf into v_0_0_2 Mar 1, 2022
@dsprayberry dsprayberry deleted the add-retry-logic branch March 1, 2022 19:09
@dsprayberry dsprayberry mentioned this pull request Mar 2, 2022
2 tasks
dsprayberry added a commit that referenced this pull request Mar 2, 2022
* Store bookmarks with only a midnight time value

* Add Changelog

* Cleanup bookmarks test

* increase no-output timeout

* address feedback in PR

* Add retry logic (#22)

* Add retry logic to the search queries

Reference code https://github.com/googleads/google-ads-python/blob/7ee4523d76a159f072541146339e7cb2721a3054/examples/misc/set_custom_client_timeouts.py#L106-L125

* Add error retry and log failed query on exception

* fix bookmarks test

* Remove selected fields log lines

* Silence google's error logging

* Prevent backoff from logging google's verbose errors

* Deduplicate log critical lines, remove unnecessary error logging

* Whitespace clean up

* Make pylint happy

* Fix field exclusion

Metrics and segments are the only fields that have exclusions, but those
exclusions can include attributes. And because we have the logic to prefer
using the root resource name when checking for `selectable_with`, we don't
need to worry about filtering out resources.

* Rename `adgroup` to `ad_group` and `audience_performance_report` to
`ad_group_audience_performance_report`. Add `campaign_audience_performance_report`

* Remove comments

* Bump to v0.1.0, update changelog

* Update tests to expect new streams

* Remove audience_performance_report, update comments

* Exclude `ad_group_audience_performance_report` from tests

* Renaming streams in discovery test

* Making tests happy w/ new stream names

* More test stream renaming. Rename confusing Class name to be accurate.

* Attempt to catch another Google Internal Error

* Make attribute exclusions mutual b/c bidding_strategy

* Link PR in Changelog

Co-authored-by: dylan-stitch <[email protected]>
Co-authored-by: kspeer <[email protected]>
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.

2 participants