Skip to content

Commit

Permalink
catch all requests exceptions to retry (#4865)
Browse files Browse the repository at this point in the history
* catch all requests exceptions to retry

* add changelog
  • Loading branch information
emmyoop authored Mar 15, 2022
1 parent d1bcff8 commit 5b01cc0
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 37 deletions.
8 changes: 8 additions & 0 deletions .changes/unreleased/Fixes-20220315-105331.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
kind: Fixes
body: Catch all Requests Exceptions on deps install to attempt retries. Also log
the exceptions hit.
time: 2022-03-15T10:53:31.637963-05:00
custom:
Author: emmyoop
Issue: "4849"
PR: "4865"
12 changes: 11 additions & 1 deletion core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2346,7 +2346,7 @@ def message(self) -> str:
class RetryExternalCall(DebugLevel):
attempt: int
max: int
code: str = "Z045"
code: str = "M020"

def message(self) -> str:
return f"Retrying external call. Attempt: {self.attempt} Max attempts: {self.max}"
Expand Down Expand Up @@ -2384,6 +2384,15 @@ def message(self) -> str:
return "Internal event buffer full. Earliest events will be dropped (FIFO)."


@dataclass
class RecordRetryException(DebugLevel):
exc: Exception
code: str = "M021"

def message(self) -> str:
return f"External call exception: {self.exc}"


# since mypy doesn't run on every file we need to suggest to mypy that every
# class gets instantiated. But we don't actually want to run this code.
# making the conditional `if False` causes mypy to skip it as dead code so
Expand Down Expand Up @@ -2737,3 +2746,4 @@ def message(self) -> str:
GeneralWarningMsg(msg="", log_fmt="")
GeneralWarningException(exc=Exception(""), log_fmt="")
EventBufferFull()
RecordRetryException(exc=Exception(""))
14 changes: 7 additions & 7 deletions core/dbt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from contextlib import contextmanager
from dbt.exceptions import ConnectionException
from dbt.events.functions import fire_event
from dbt.events.types import RetryExternalCall
from dbt.events.types import RetryExternalCall, RecordRetryException
from dbt import flags
from enum import Enum
from typing_extensions import Protocol
Expand Down Expand Up @@ -600,19 +600,19 @@ def __contains__(self, name) -> bool:

def _connection_exception_retry(fn, max_attempts: int, attempt: int = 0):
"""Attempts to run a function that makes an external call, if the call fails
on a connection error, timeout or decompression issue, it will be tried up to 5 more times.
See https://github.com/dbt-labs/dbt-core/issues/4579 for context on this decompression issues
specifically.
on a Requests exception or decompression issue (ReadError), it will be tried
up to 5 more times. All exceptions that Requests explicitly raises inherit from
requests.exceptions.RequestException. See https://github.com/dbt-labs/dbt-core/issues/4579
for context on this decompression issues specifically.
"""
try:
return fn()
except (
requests.exceptions.ConnectionError,
requests.exceptions.Timeout,
requests.exceptions.ContentDecodingError,
requests.exceptions.RequestException,
ReadError,
) as exc:
if attempt <= max_attempts - 1:
fire_event(RecordRetryException(exc=exc))
fire_event(RetryExternalCall(attempt=attempt, max=max_attempts))
time.sleep(1)
_connection_exception_retry(fn, max_attempts, attempt + 1)
Expand Down
41 changes: 12 additions & 29 deletions test/unit/test_core_dbt_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,17 @@ def test_connection_exception_retry_none(self):
connection_exception_retry(lambda: Counter._add(), 5)
self.assertEqual(1, counter)

def test_connection_exception_retry_success_requests_exception(self):
Counter._reset()
connection_exception_retry(lambda: Counter._add_with_requests_exception(), 5)
self.assertEqual(2, counter) # 2 = original attempt returned None, plus 1 retry

def test_connection_exception_retry_max(self):
Counter._reset()
with self.assertRaises(ConnectionException):
connection_exception_retry(lambda: Counter._add_with_exception(), 5)
self.assertEqual(6, counter) # 6 = original attempt plus 5 retries

def test_connection_exception_retry_success(self):
Counter._reset()
connection_exception_retry(lambda: Counter._add_with_limited_exception(), 5)
self.assertEqual(2, counter) # 2 = original attempt plus 1 retry

def test_connection_timeout(self):
Counter._reset()
connection_exception_retry(lambda: Counter._add_with_timeout(), 5)
self.assertEqual(2, counter) # 2 = original attempt plus 1 retry

def test_connection_exception_retry_success_none_response(self):
Counter._reset()
connection_exception_retry(lambda: Counter._add_with_none_exception(), 5)
self.assertEqual(2, counter) # 2 = original attempt returned None, plus 1 retry

def test_connection_exception_retry_success_failed_untar(self):
Counter._reset()
connection_exception_retry(lambda: Counter._add_with_untar_exception(), 5)
Expand All @@ -44,25 +34,18 @@ class Counter():
def _add():
global counter
counter+=1
def _add_with_exception():
global counter
counter+=1
raise requests.exceptions.ConnectionError
def _add_with_limited_exception():
global counter
counter+=1
if counter < 2:
raise requests.exceptions.ConnectionError
def _add_with_timeout():
# All exceptions that Requests explicitly raises inherit from
# requests.exceptions.RequestException so we want to make sure that raises plus one exception
# that inherit from it for sanity
def _add_with_requests_exception():
global counter
counter+=1
if counter < 2:
raise requests.exceptions.Timeout
def _add_with_none_exception():
raise requests.exceptions.RequestException
def _add_with_exception():
global counter
counter+=1
if counter < 2:
raise requests.exceptions.ContentDecodingError
raise requests.exceptions.ConnectionError
def _add_with_untar_exception():
global counter
counter+=1
Expand Down
1 change: 1 addition & 0 deletions test/unit/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ def MockNode():
IntegrationTestError(msg=''),
IntegrationTestException(msg=''),
EventBufferFull(),
RecordRetryException(exc=Exception('')),
UnitTestInfo(msg=''),
]

Expand Down

0 comments on commit 5b01cc0

Please sign in to comment.