Backport 5137 5069 (#5147)

* move deprecation check outside package caching (#5069)

* move deprecation check outside package caching

* add changelog

* fix retry logic failures (#5137)

* fix retry logic failures

* changelog

* add tests to make sure data is getting where it needs to

* rename file

* remove duplicate file

* move unit test to old framework since new one doesn't exist here

.changes/unreleased/Fixes-20220422-131227.yaml

@@ -0,0 +1,7 @@
+kind: Fixes
+body: Fix retry logic to return values after initial try
+time: 2022-04-22T13:12:27.239055-05:00
+custom:
+  Author: emmyoop
+  Issue: "5023"
+  PR: "5137"

.changes/unreleased/Under the Hood-20220414-132206.yaml

@@ -0,0 +1,7 @@
+kind: Under the Hood
+body: Move package deprecation check outside of package cache
+time: 2022-04-14T13:22:06.157579-05:00
+custom:
+  Author: emmyoop
+  Issue: "5068"
+  PR: "5069"

core/dbt/clients/registry.py

@@ -89,6 +89,15 @@ def _get(package_name, registry_base_url=None):
         fire_event(RegistryResponseExtraNestedKeys(response=response))
         raise requests.exceptions.ContentDecodingError(error_msg, response=resp)
 
+    return response
+
+
+_get_cached = memoized(_get_with_retries)
+
+
+def package(package_name, registry_base_url=None) -> Dict[str, Any]:
+    # returns a dictionary of metadata for all versions of a package
+    response = _get_cached(package_name, registry_base_url)
     # Either redirectnamespace or redirectname in the JSON response indicate a redirect
     # redirectnamespace redirects based on package ownership
     # redirectname redirects based on package name
@@ -107,16 +116,6 @@ def _get(package_name, registry_base_url=None):
 
         new_nwo = use_namespace + "/" + use_name
         deprecations.warn("package-redirect", old_name=package_name, new_name=new_nwo)
-
-    return response
-
-
-_get_cached = memoized(_get_with_retries)
-
-
-def package(package_name, registry_base_url=None) -> Dict[str, Any]:
-    # returns a dictionary of metadata for all versions of a package
-    response = _get_cached(package_name, registry_base_url)
     return response["versions"]
 
 

core/dbt/utils.py

@@ -614,6 +614,6 @@ def _connection_exception_retry(fn, max_attempts: int, attempt: int = 0):
             fire_event(RecordRetryException(exc=exc))
             fire_event(RetryExternalCall(attempt=attempt, max=max_attempts))
             time.sleep(1)
-            _connection_exception_retry(fn, max_attempts, attempt + 1)
+            return _connection_exception_retry(fn, max_attempts, attempt + 1)
         else:
             raise ConnectionException('External connection exception occurred: ' + str(exc))

test/unit/test_connection_retries.py

@@ -0,0 +1,59 @@
+import functools
+import pytest
+from requests.exceptions import RequestException
+from dbt.exceptions import ConnectionException
+from dbt.utils import _connection_exception_retry
+
+
+def no_retry_fn():
+    return "success"
+
+
+class TestNoRetries:
+    def test_no_retry(self):
+        fn_to_retry = functools.partial(no_retry_fn)
+        result = _connection_exception_retry(fn_to_retry, 3)
+
+        expected = "success"
+
+        assert result == expected
+
+
+def no_success_fn():
+    raise RequestException("You'll never pass")
+    return "failure"
+
+
+class TestMaxRetries:
+    def test_no_retry(self):
+        fn_to_retry = functools.partial(no_success_fn)
+
+        with pytest.raises(ConnectionException):
+            _connection_exception_retry(fn_to_retry, 3)
+
+
+def single_retry_fn():
+    global counter
+    if counter == 0:
+        counter += 1
+        raise RequestException("You won't pass this one time")
+    elif counter == 1:
+        counter += 1
+        return "success on 2"
+
+    return "How did we get here?"
+
+
+class TestSingleRetry:
+    def test_no_retry(self):
+        global counter
+        counter = 0
+
+        fn_to_retry = functools.partial(single_retry_fn)
+        result = _connection_exception_retry(fn_to_retry, 3)
+        expected = "success on 2"
+
+        # We need to test the return value here, not just that it did not throw an error.
+        # If the value is not being passed it causes cryptic errors
+        assert result == expected
+        assert counter == 2