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

UPDATE _is_retryable() to handle BQ rateLimitExceeded #2796

Merged

Conversation

championj-foxtel
Copy link
Contributor

@championj-foxtel championj-foxtel commented Sep 29, 2020

ADDED test case for rateLimitExceeded in test_is_retryable()

resolves #2795

Description

This PR enables attempted retry on a 403 rateLimitExceeded error returned by BigQuery & associated test

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@cla-bot cla-bot bot added the cla:yes label Sep 29, 2020
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

This looks great @championj-foxtel! Just some tiny comments about the changelog entries.

If you're up for a simple rebase + moving the changelog entries, I think we could sneak this into v0.18.1, for which we'll have a release candidate quite soon. Otherwise, this will be shipping with v0.19.0 in late October.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@championj-foxtel championj-foxtel changed the base branch from dev/kiyoshi-kuromiya to dev/0.18.1 September 29, 2020 23:55
@championj-foxtel
Copy link
Contributor Author

Hi @jtcohen6, while doing the rebase onto dev/0.18.1 the CHANGELOG.md got a bit messed up. As the changes are small I took a new cut off dev/0.18.1 and replicated the change.

I have seen that the unit tests (make test-unit) are failing on branch dev/0.18.1 and are therefore now failing on this branch - is this the behaviour you see when running the same? I can see ci/circleci: unit check failed which I would guess is this same test.

@jtcohen6
Copy link
Contributor

@championj-foxtel Thanks for the branch flexibility! I'm not sure why unit-py36 failed on the first go round, but it passed locally and when I reran in Circle. So long as the rest of the integration tests pass, I'll merge this in shortly.

@jtcohen6 jtcohen6 merged commit 139b353 into dbt-labs:dev/0.18.1 Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BigQuery] Retry on rateLimitExceeded response
2 participants