Skip to content

Commit

Permalink
[Backport] 4865 dep retries (#4867)
Browse files Browse the repository at this point in the history
* catch all requests exceptions to retry (#4865)
* catch all requests exceptions to retry

* add changelog

* fixed pre-1.1 serialization issues
  • Loading branch information
emmyoop authored Mar 16, 2022
1 parent 7fca9ec commit 2748e4b
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 38 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 @@ -2381,7 +2381,7 @@ def message(self) -> str:
class RetryExternalCall(DebugLevel, Cli, File):
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 @@ -2419,6 +2419,15 @@ def message(self) -> str:
return "Internal event buffer full. Earliest events will be dropped (FIFO)."


@dataclass
class RecordRetryException(DebugLevel, Cli, File):
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 @@ -2774,3 +2783,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 @@ -16,7 +16,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 enum import Enum
from typing_extensions import Protocol
from typing import (
Expand Down Expand Up @@ -599,19 +599,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
3 changes: 2 additions & 1 deletion test/unit/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,8 @@ def MockNode():
IntegrationTestError(''),
IntegrationTestException(''),
EventBufferFull(),
UnitTestInfo('')
RecordRetryException(Exception('')),
UnitTestInfo(''),
]


Expand Down

0 comments on commit 2748e4b

Please sign in to comment.