googleapis / python-bigquery

Apache License 2.0
746 stars 306 forks source link

Should a rateLimitExceeded have a "429 Too Many Requests" instead of "403 Forbidden"? #1985

Closed cozos closed 3 months ago

cozos commented 3 months ago

In many cases, users want to be able to retry rateLimitExceeded errors. Many retry libraries and codebases support this by automatically retrying HTTP exceptions with a 429 Too Many Requests HTTP error, and any 5XX error codes. For example, BigQuery itself. However BigQuery will raise a 403 Forbidden for rate limit problems - see: https://github.com/googleapis/python-bigquery/blob/f55864ec3d6381f2b31598428a64822fdc73cb56/google/cloud/bigquery/job/base.py#L49

A 403 Forbidden implies that the API user does not have the permissions or authentication credentials to do the thing. That seems inappropriate for a rateLimitError, right?

This mis-coding has two consequences for retry logic:

chalmerlowe commented 3 months ago

A few quick notes for future me (or anyone else picking this up).

We are looking at three different sources of errors here.

Each of these libraries covers different classes of errors.

It appears that the error we are most interested in raising is exceptions.TooManyRequests, from the google-api-core.exceptions module. We reference it in the [BigQuery retry.py file] after importing exceptions from python-api-core: (https://github.com/googleapis/python-bigquery/blob/4383cfe7c7571ffaa3efd5e45ca33c6ff978f274/google/cloud/bigquery/retry.py#L26-L36).