Repository: googleapis/python-bigquery (language: python)

Issue:
Should a rateLimitExceeded have a "429 Too Many Requests" instead of "403 Forbidden"?
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](https://github.com/googleapis/python-bigquery/blob/4383cfe7c7571ffaa3efd5e45ca33c6ff978f274/google/cloud/bigquery/retry.py#L26-L36). 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:

* In codebases where you'd expect to get retries for free, you don't, because libraries won't retry a `403 Forbidden`, and rightly so - why would you retry an access error.
* Users who do want to implement retries for BQ `rateLimitExceeded` errors now have to retry on `403 Forbidden`, possibly causing them to retry genuine permissions/auth errors that will fail no matter how many attempts.


Discussion / hints:
A few quick notes for future me (or anyone else picking this up).

We are looking at three different sources of errors here.

* Errors native to Python in the `http` Standard Library (in [base.py](https://github.com/googleapis/python-bigquery/blob/f55864ec3d6381f2b31598428a64822fdc73cb56/google/cloud/bigquery/job/base.py#L49))
* Errors native to the `requests` module (in [retry.py](https://github.com/googleapis/python-bigquery/blob/4383cfe7c7571ffaa3efd5e45ca33c6ff978f274/google/cloud/bigquery/retry.py#L26-L36))
* Errors native to the GoogleAPIs `python-api-core` module (in [retry.py](https://github.com/googleapis/python-bigquery/blob/4383cfe7c7571ffaa3efd5e45ca33c6ff978f274/google/cloud/bigquery/retry.py#L26-L36))

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).




Fix patch:
diff --git a/google/cloud/bigquery/job/base.py b/google/cloud/bigquery/job/base.py
index 6f9726181..f165fd036 100644
--- a/google/cloud/bigquery/job/base.py
+++ b/google/cloud/bigquery/job/base.py
@@ -49,7 +49,7 @@
     "notImplemented": http.client.NOT_IMPLEMENTED,
     "policyViolation": http.client.FORBIDDEN,
     "quotaExceeded": http.client.FORBIDDEN,
-    "rateLimitExceeded": http.client.FORBIDDEN,
+    "rateLimitExceeded": http.client.TOO_MANY_REQUESTS,
     "resourceInUse": http.client.BAD_REQUEST,
     "resourcesExceeded": http.client.BAD_REQUEST,
     "responseTooLarge": http.client.FORBIDDEN,


Test patch:
diff --git a/tests/unit/test_job_retry.py b/tests/unit/test_job_retry.py
index 298ab9a56..958986052 100644
--- a/tests/unit/test_job_retry.py
+++ b/tests/unit/test_job_retry.py
@@ -442,7 +442,7 @@ def api_request(method, path, query_params=None, data=None, **kw):
 
     orig_job_id = job.job_id
     job_retry = dict(job_retry=None) if job_retry_on_query == "Result" else {}
-    with pytest.raises(google.api_core.exceptions.Forbidden):
+    with pytest.raises(google.api_core.exceptions.TooManyRequests):
         job.result(**job_retry)
 
     assert job.job_id == orig_job_id