Repository: dbt-labs/dbt-bigquery (language: python)

Issue:
[ADAP-498] [Bug] BQ does not retry on 503
### Is this a new bug in dbt-bigquery?

- [X] I believe this is a new bug in dbt-bigquery
- [X] I have searched the existing issues, and I could not find an existing issue for this bug

### Current Behavior

Current if BigQuery returns a 503 error we do not retry even though BigQuery recommends that as the course of action.

### Expected Behavior

This is not a regression but rather an oversight -- 503 errors should be both retryable and reopenable since it indicates a transient unavailable condition in BigQuery

### Steps To Reproduce

Transient -- requires intermittent error from BQ.

### Relevant log output

_No response_

### Environment

```markdown
- dbt-core: all 
- dbt-bigquery: all
```


### Additional Context

_No response_


Discussion / hints:
Thanks for reaching out @barberscott !

We'll put this in our queue.

The solution might be as simple as adding [`google.cloud.exceptions.ServiceUnavailable`](https://github.com/googleapis/python-api-core/blob/2a6a36bd820e1344c8b2526eb75d0b9f3a055d3b/google/api_core/exceptions.py#L409-L414) to the list here:

https://github.com/dbt-labs/dbt-bigquery/blob/7c216445f8009baa9cec4d61dd56693be1dd79fa/dbt/adapters/bigquery/connections.py#L53-L59
This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.
Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest. Just add a comment to notify the maintainers.
@dbeatty10 
I created an ServiceUnavailable instance and ran the test code ([test_is_retryable](https://github.com/dbt-labs/dbt-bigquery/blob/06851679f75d18ece98c95d4eb2a0ddd16544f4d/tests/unit/test_bigquery_connection_manager.py#L81)).

Current: Not added ServiceUnavailable on `RETRYABLE_ERRORS`.
Result: Test passed.

```
def test_is_retrievable(self):
        _is_retryable = dbt.adapters.bigquery.connections._is_retryable
        exceptions = dbt.adapters.bigquery.impl.google.cloud.exceptions
        Internal Server Error = Exceptions.Internal Server Error ("Code Abort")
        bad_request_error = Exception.BadRequest("Code is broken")
        connection_error = connection_error("Code broke")
        client_error = Exception.ClientError("Invalid code")
        rate_limit_error = Exception.Forbidden(
            "Code is broken", error=[{"reason": "rateLimitExceeded"}]]
        )
        # add service_unavailable_error
        service_unavailable_error = Exception.ServiceUnavailable("Code is broken")

        self.assertTrue(_is_retryable(internal_server_error))
        self.assertTrue(_is_retryable(bad_request_error))
        self.assertTrue(_is_retryable(connection_error))
        self.assertFalse(_is_retryable(client_error))
        self.assertTrue(_is_retryable(rate_limit_error))
        # passed below assertion
        self.assertTrue(_is_retryable(service_unavailable_error))
```

https://github.com/dbt-labs/dbt-bigquery/blob/06851679f75d18ece98c95d4eb2a0ddd16544f4d/dbt/adapters/bigquery/connections.py#L57-L63

The ServiceUnavailable class inherits from the ServerError class, so it seems to pass above test.
I'd like to fix this, but is there anything else I look at? 🙏
Adding it to the [`test_is_retryable`](https://github.com/dbt-labs/dbt-bigquery/blob/06851679f75d18ece98c95d4eb2a0ddd16544f4d/tests/unit/test_bigquery_connection_manager.py#L81) test like that makes sense 👍

But ... the thing that is surprising to me: if [`ServiceUnavailable`](https://github.com/googleapis/python-api-core/blob/2a6a36bd820e1344c8b2526eb75d0b9f3a055d3b/google/api_core/exceptions.py#L409-L414) inherits from `ServerError` and your modified test passes, then why is this not being retried?

Is is possible that the BigQuery client is raising a different error class for 503 errors other than `ServiceUnavailable`?

@jx2lee Do you happen to have any python stacktraces available where you ran into this problem and dbt-bigquery didn't retry?
@dbeatty10 

> Is is possible that the BigQuery client is raising a different error class for 503 errors other than ServiceUnavailable?

no, i expected `it's impossible.`
we can create error classes with the [from_http_status](https://github.com/googleapis/python-api-core/blob/17ff5f1d83a9a6f50a0226fb0e794634bd584f17/google/api_core/exceptions.py#L452) and [from_grpc_status](https://github.com/googleapis/python-api-core/blob/17ff5f1d83a9a6f50a0226fb0e794634bd584f17/google/api_core/exceptions.py#L532) functions. ([google.api_core.exceptions](https://github.com/googleapis/python-api-core/blob/main/google/api_core/exceptions.py)). error class generated from this functions always be "ServiceUnavailable"

<br>
<br>

> Do you happen to have any python stacktraces available where you ran into this problem and dbt-bigquery didn't retry?

That issue has never been occured...🙃 
I need to more detailed logs when it happened. 

IMO, If the issue reporter can't provide more error logs, I think okay to close the issue.

- 503 code does not return any error class other than `ServiceUnavailable`
- The functions that raising error in the googleapis package only generate the `ServiceUnavailable`
@dbeatty10 
Is there anything else should check?
We did hit this recently. We use [external-tables](https://github.com/dbt-labs/dbt-external-tables) on a `on-run-start` macro. We also use service account impersonation in the dbt profile. While running dbt docs generate on CI environment we got:

```
('Unable to acquire impersonated credentials', '{\n  "error": {\n    "code": 503,\n    "message": "Authentication backend unavailable.",\n    "status": "UNAVAILABLE"\n  }\n}\n')
```

Because this happens intermittently on an isolated system, I don't have more logs.
Thanks for this report @rrbarbosa !

Since this is intermittent (and maybe relatively rare also), it has been hard to nail down.

If anyone can provide information to suggest that dbt is **not** retrying at least once, that would be very helpful 🙏
@jx2lee -- would you be willing to raise a PR with the addition you made to this test case?

I think that would be sufficient for us to establish that the `ServiceUnavailable` is retryable (which would allow us to close this issue).
@dbeatty10 okay, i would create PR included above test code soon!

Fix patch:
diff --git a/.changes/unreleased/Features-20240505-011838.yaml b/.changes/unreleased/Features-20240505-011838.yaml
new file mode 100644
index 000000000..66411853f
--- /dev/null
+++ b/.changes/unreleased/Features-20240505-011838.yaml
@@ -0,0 +1,6 @@
+kind: Features
+body: add is_retryable test case when raise ServiceUnavailable
+time: 2024-05-05T01:18:38.737882+09:00
+custom:
+  Author: jx2lee
+  Issue: "682"


Test patch:
diff --git a/tests/unit/test_bigquery_connection_manager.py b/tests/unit/test_bigquery_connection_manager.py
index d09cb1635..1c14100f6 100644
--- a/tests/unit/test_bigquery_connection_manager.py
+++ b/tests/unit/test_bigquery_connection_manager.py
@@ -84,12 +84,14 @@ def test_is_retryable(self):
         rate_limit_error = exceptions.Forbidden(
             "code broke", errors=[{"reason": "rateLimitExceeded"}]
         )
+        service_unavailable_error = exceptions.ServiceUnavailable("service is unavailable")
 
         self.assertTrue(_is_retryable(internal_server_error))
         self.assertTrue(_is_retryable(bad_request_error))
         self.assertTrue(_is_retryable(connection_error))
         self.assertFalse(_is_retryable(client_error))
         self.assertTrue(_is_retryable(rate_limit_error))
+        self.assertTrue(_is_retryable(service_unavailable_error))
 
     def test_drop_dataset(self):
         mock_table = Mock()