From 3817ad853f35418c0e6d8cb43bed83ebbf60f0bf Mon Sep 17 00:00:00 2001 From: James Gilmore Date: Thu, 23 Jan 2025 10:15:59 +0000 Subject: [PATCH 1/5] Add `urllib3` to requirements for Retry support This is needed since the addition of a Retry strategy which is provided by the `urllib3` library. This supports both PY2 (if we're really honestly still supporting this) and PY3 and was added since v1.9.0, so just pinning this to that version just in case someone is using an ancient (11+ years old) version of urllib3. --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 6a9edd971..97dd86abb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,3 +3,4 @@ six >= 1.7.3 curlify >= 2.1.0 pycountry >= 19.8.18 aiohttp; python_version >= '3.5.3' +urllib3 >= 1.9.0 # Minimum version that supports `Retry`. From 996401aa355c85ea99ed4012db5dec5b53af49de Mon Sep 17 00:00:00 2001 From: James Gilmore Date: Thu, 23 Jan 2025 10:17:47 +0000 Subject: [PATCH 2/5] Add a default retry strategy and config support To provide consistency with how an API call can be configured in this library, wanted to add the ability to configure a blanket retry strategy at the config level if a user wants all API calls (not POST etc.) to have a retry strategy. --- facebook_business/apiconfig.py | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/facebook_business/apiconfig.py b/facebook_business/apiconfig.py index 8e770a3f7..52f615025 100644 --- a/facebook_business/apiconfig.py +++ b/facebook_business/apiconfig.py @@ -3,9 +3,41 @@ # This source code is licensed under the license found in the # LICENSE file in the root directory of this source tree. +from six.moves import http_client +from urllib3 import Retry + ads_api_config = { 'API_VERSION': 'v21.0', 'SDK_VERSION': 'v21.0.5', - 'STRICT_MODE': False + 'STRICT_MODE': False, + + # Whether to enable a retry strategy on any API calls being made. When set + # to True, a default strategy is used, which is also configurable in this + # config. + 'RETRY_MODE': False, + 'RETRY_STRATEGY': { + 'DEFAULT_RETRIES': 5, + 'DEFAULT_BACKOFF_FACTOR': 0.5, # Time doubles between API calls. + 'RETRY_HTTP_CODES': [ + http_client.REQUEST_TIMEOUT, + http_client.TOO_MANY_REQUESTS, + http_client.INTERNAL_SERVER_ERROR, + http_client.SERVICE_UNAVAILABLE, + http_client.GATEWAY_TIMEOUT, + ], + } + } + + +def get_default_retry_strategy(): + """Gets the default retry strategy, based on the API config.""" + retry_config = ads_api_config['RETRY_STRATEGY'] + return Retry( + total=retry_config["DEFAULT_RETRIES"], + status_forcelist=retry_config["RETRY_HTTP_CODES"], + backoff_factor=retry_config["DEFAULT_BACKOFF_FACTOR"], + raise_on_status=False, # To allow consistent handling of response. + respect_retry_after_header=True, + ) From d63009aba4d5b512464323b1e3bc9b36c1bf9b63 Mon Sep 17 00:00:00 2001 From: James Gilmore Date: Thu, 23 Jan 2025 10:19:21 +0000 Subject: [PATCH 3/5] Update FacebookSession to allow injecting a Retry. The alternative way to handle retries is to specify your own at the session level. This provides full control over how to do retries, but more at the owners own risk (as we've already provided them with a default solution). --- facebook_business/session.py | 40 ++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/facebook_business/session.py b/facebook_business/session.py index 3dad0d9ec..058374ea3 100644 --- a/facebook_business/session.py +++ b/facebook_business/session.py @@ -13,10 +13,12 @@ import requests import os +from facebook_business.apiconfig import get_default_retry_strategy, ads_api_config + class FacebookSession(object): """ - FacebookSession manages the the Graph API authentication and https + FacebookSession manages the Graph API authentication and https connection. Attributes: @@ -26,13 +28,16 @@ class FacebookSession(object): access_token: The access token. appsecret_proof: The application secret proof. proxies: Object containing proxies for 'http' and 'https' + retry_strategy (Optional[urllib3.Retry]): A optional retry strategy to + apply to the API call. If `RETRY_MODE` is True in the `apiconfig` + then we'll use a default Retry strategy. requests: The python requests object through which calls to the api can be made. """ GRAPH = 'https://graph.facebook.com' def __init__(self, app_id=None, app_secret=None, access_token=None, - proxies=None, timeout=None, debug=False): + proxies=None, timeout=None, retry_strategy=None, debug=False): """ Initializes and populates the instance attributes with app_id, app_secret, access_token, appsecret_proof, proxies, timeout and requests @@ -59,6 +64,8 @@ def __init__(self, app_id=None, app_secret=None, access_token=None, if self.proxies: self.requests.proxies.update(self.proxies) + self.retry_strategy = self._mount_retry_strategy(retry_strategy) + def _gen_appsecret_proof(self): h = hmac.new( self.app_secret.encode('utf-8'), @@ -69,4 +76,33 @@ def _gen_appsecret_proof(self): self.appsecret_proof = h.hexdigest() return self.appsecret_proof + def _mount_retry_strategy(self, retry_strategy): + """ + Mounts any available retry strategy to the request's session. + + Provides ability to fully specify a Retry strategy, or if RETRY_MODE + is set on the API Config, then a default retry strategy will be used, + which is partially configurable. + + Attributes: + retry_strategy (Optional[urllib3.Retry]): The retry strategy to + apply to the session and will be used for all API calls + against the session. + """ + retry_mode = ads_api_config["RETRY_MODE"] + if retry_mode and not retry_strategy: + retry_strategy = get_default_retry_strategy() + + # Return early if no retry strategy was found. + if not retry_strategy: + return + + # Inject the Retry strategy into the session directly. + adapter = requests.adapters.HTTPAdapter(max_retries=retry_strategy) + self.requests.mount("https://", adapter) + self.requests.mount("http://", adapter) + + return retry_strategy + + __all__ = ['FacebookSession'] From 8db5c996c982fd9255e7539c0ce4de1a9242de83 Mon Sep 17 00:00:00 2001 From: James Gilmore Date: Thu, 23 Jan 2025 10:21:35 +0000 Subject: [PATCH 4/5] Update `init` for FacebookAdsApi to add Retry Add in the injection point to the FacebookAdsApi to allow manually passing in a custom retry strategy which is then passed into the FacebookSession and finally into the requests.Session object. --- facebook_business/api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/facebook_business/api.py b/facebook_business/api.py index 095a9a3e7..352f7afac 100644 --- a/facebook_business/api.py +++ b/facebook_business/api.py @@ -187,9 +187,10 @@ def init( timeout=None, debug=False, crash_log=True, + retry_strategy=None, ): session = FacebookSession(app_id, app_secret, access_token, proxies, - timeout) + timeout, retry_strategy=retry_strategy) api = cls(session, api_version, enable_debug_logger=debug) cls.set_default_api(api) From 23571fd4162d92e15a720c57658b9af930734b90 Mon Sep 17 00:00:00 2001 From: James Gilmore Date: Thu, 23 Jan 2025 10:22:45 +0000 Subject: [PATCH 5/5] Add integration tests for retry strategy. Uses a new test library, responses, which allows you to better test making multiple API calls and therefore testing your Retry strategy actually works. --- facebook_business/test/integration_api.py | 163 ++++++++++++++++++++++ requirements-tests.txt | 1 + 2 files changed, 164 insertions(+) create mode 100644 facebook_business/test/integration_api.py diff --git a/facebook_business/test/integration_api.py b/facebook_business/test/integration_api.py new file mode 100644 index 000000000..eba70043b --- /dev/null +++ b/facebook_business/test/integration_api.py @@ -0,0 +1,163 @@ +import unittest +from unittest.mock import patch + +import responses +from six.moves import http_client +from urllib3 import Retry + +from facebook_business import FacebookAdsApi, apiconfig +from facebook_business.apiconfig import get_default_retry_strategy +from facebook_business.exceptions import FacebookRequestError +from facebook_business.test.integration_utils import IntegrationTestCase + + +class FacebookAdsApiTestCase(IntegrationTestCase): + + def setUp(self): + """ + We're accessing the low-level parts of the API functionality here, so don't want to mock + the requests in the same way, but want to at least partially conform. + """ + self.facebook_ads_api = FacebookAdsApi.init(access_token='access_token', crash_log=False) + self.facebook_ads_api_retry = FacebookAdsApi.init( + access_token='access_token', + crash_log=False, + retry_strategy=get_default_retry_strategy() + ) + self.url = "http://facebook.com/some/path" + + def tearDown(self): + ... + + @responses.activate + def test_is_success_200(self): + """ + Simple test to show the API call will respond with a 200 status code + """ + # Arrange - Override the low-level API calls. We just need to make sure these return 200 + # as no real API calls should be made. + responses.add(responses.GET, self.url, status=http_client.OK) + + # Act + facebook_response = self.facebook_ads_api.call(method="GET", path=self.url) + + # Assert + self.assertEqual(facebook_response.status(), http_client.OK) + self.assertTrue(facebook_response.is_success()) + + @responses.activate + def test_failure_raised_after_service_unavailable(self): + """ + Tests that the API call will raise an error when getting a non 2xx error code. + + Default is without a Retry strategy. + """ + # Arrange - Override the low-level API calls. Make sure we start with a 500 then a 200. + responses.add(responses.GET, self.url, status=http_client.INTERNAL_SERVER_ERROR) + responses.add(responses.GET, self.url, status=http_client.OK) + + # Act + with self.assertRaises(FacebookRequestError): + self.facebook_ads_api.call(method="GET", path=self.url) + + @responses.activate + def test_success_after_service_unavailable_with_implicit_retry_strategy(self): + """ + Tests that the API call will return a 200 after an initial service issue. + + Using the default retry strategy. + """ + # Arrange - Override the low-level API calls. Make sure we start with a 500 then a 200. + responses.add(responses.GET, self.url, status=http_client.INTERNAL_SERVER_ERROR) + responses.add(responses.GET, self.url, status=http_client.OK) + + # Act + facebook_response = self.facebook_ads_api_retry.call(method="GET", path=self.url) + + # Assert + self.assertEqual(facebook_response.status(), http_client.OK) + self.assertTrue(facebook_response.is_success()) + + @responses.activate + @patch.dict(apiconfig.ads_api_config["RETRY_STRATEGY"], {"DEFAULT_RETRIES": 1}) + def test_failure_after_service_unavailable_more_than_default_retry_strategy_allows(self): + """ + Tests that the API call will still raise a `FacebookRequestError` after exhausting retries. + + Using the default retry strategy. + """ + facebook_ads_api_retry = FacebookAdsApi.init( + access_token='access_token', + crash_log=False, + ) + + # Arrange - Override the low-level API calls. Make sure we start with a 500 then a 200. + responses.add(responses.GET, self.url, status=http_client.INTERNAL_SERVER_ERROR) + responses.add(responses.GET, self.url, status=http_client.INTERNAL_SERVER_ERROR) + responses.add(responses.GET, self.url, status=http_client.OK) + + # Ac + with self.assertRaises(FacebookRequestError): + facebook_ads_api_retry.call(method="GET", path=self.url) + + @responses.activate + def test_success_after_service_unavailable_with_explicit_retry_strategy(self): + """ + Tests that the API call will return a 200 after an initial service issue. + + Using ta custom retry strategy. + """ + # Arrange - Define the custom retry. + retry_strategy = Retry( + total=1, + status_forcelist=[http_client.INTERNAL_SERVER_ERROR], + raise_on_status=False, # To allow consistent handling of response. + ) + facebook_ads_api_retry = FacebookAdsApi.init( + access_token='access_token', + crash_log=False, + retry_strategy=retry_strategy + ) + + # Arrange - Override the low-level API calls. Make sure we start with a 500 then a 200. + responses.add(responses.GET, self.url, status=http_client.INTERNAL_SERVER_ERROR) + responses.add(responses.GET, self.url, status=http_client.OK) + + # Act + facebook_response = facebook_ads_api_retry.call(method="GET", path=self.url) + + # Assert + self.assertEqual(facebook_response.status(), http_client.OK) + self.assertTrue(facebook_response.is_success()) + + @responses.activate + def test_failure_after_service_unavailable_more_than_explicit_retry_strategy_allows(self): + """ + Tests that the API call will still raise a `FacebookRequestError` after exhausting retries. + + Using a custom retry strategy. + """ + # Arrange - Define the custom retry. + retry_strategy = Retry( + total=1, + status_forcelist=[http_client.INTERNAL_SERVER_ERROR], + raise_on_status=False, # To allow consistent handling of response. + ) + facebook_ads_api_retry = FacebookAdsApi.init( + access_token='access_token', + crash_log=False, + retry_strategy=retry_strategy + ) + + # Arrange - Override the low-level API calls. Make sure we start with a 500 then a 200. + responses.add(responses.GET, self.url, status=http_client.INTERNAL_SERVER_ERROR) + responses.add(responses.GET, self.url, status=http_client.INTERNAL_SERVER_ERROR) + responses.add(responses.GET, self.url, status=http_client.OK) + + # Ac + with self.assertRaises(FacebookRequestError): + facebook_ads_api_retry.call(method="GET", path=self.url) + + +if __name__ == '__main__': + unittest.main() diff --git a/requirements-tests.txt b/requirements-tests.txt index df1eda23f..38805b825 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -1 +1,2 @@ mock >= 1.0.1 +responses >= 0.25.6 \ No newline at end of file