Merge pull request #34 from adobe-apiplatform/v2

Fix #32 and other small enhancements

Author: adobeDan

.gitignore

@@ -1,6 +1,7 @@
 # platform artifacts
 .*
 !.travis.yml
+!.gitignore
 
 # build and scratch areas
 local/

HISTORY.md

@@ -10,7 +10,7 @@ Fast-follow bug fix release:
     * Update parameter names were incorrect.
     * Test mode wasn't working.
 * [Issue 28](https://github.com/adobe-apiplatform/umapi-client.py/issues/28)
-    * Reuse exisiting open connections across calls.
+    * Reuse existing open connections across calls.
 
 ### Version 2.0.2
 
@@ -21,3 +21,15 @@ Enhancement release:
 * (No Issue)
     * Add this HISTORY.md file to summarize releases
     * Add version.py file to synchronize version between setup and module.
+
+### Version 2.0.3
+
+Enhancement release:
+
+* [Issue 32](https://github.com/adobe-apiplatform/umapi-client.py/issues/32)
+    * change timeout default to 2 minutes
+    * add retry after timeout.
+    * change default create behavior to "ignoreIfAlreadyExists"
+* (No issue)
+    * fix misspellings
+    * change .gitignore so that .gitignore is not ignored

docs/index.md

@@ -40,7 +40,7 @@ extensions.
 # Getting Started
 
 Before making calls to the User Management API, you must complete
-the following prepartory steps:
+the following preparatory steps:
 
 1. Obtain admin access to an Adobe Enterprise Dashboard.
 2. Set up a private/public key pair

tests/test_connections.py

@@ -104,15 +104,17 @@ def test_post_success_test_mode():
 def test_get_timeout():
     with mock.patch("umapi_client.connection.requests.Session.get") as mock_get:
         mock_get.side_effect = requests.Timeout
-        conn = Connection(**mock_connection_params)
+        conn = Connection(**dict(mock_connection_params, retry_max_attempts=7))
         pytest.raises(UnavailableError, conn.make_call, "")
+        assert mock_get.call_count == 7
 
 
 def test_post_timeout():
     with mock.patch("umapi_client.connection.requests.Session.post") as mock_post:
         mock_post.side_effect = requests.Timeout
-        conn = Connection(**mock_connection_params)
+        conn = Connection(**dict(mock_connection_params, retry_max_attempts=2))
         pytest.raises(UnavailableError, conn.make_call, "", [3, 5])
+        assert mock_post.call_count == 2
 
 
 def test_get_retry_header_1():
@@ -187,8 +189,7 @@ def test_get_retry_logging(log_stream):
 UMAPI timeout...service unavailable (code 429 on try 2)
 Next retry in 3 seconds...
 UMAPI timeout...service unavailable (code 429 on try 3)
-Next retry in 3 seconds...
-UMAPI timeout...giving up after 3 attempts.
+UMAPI timeout...giving up after 3 attempts (6 seconds).
 """
 
 
@@ -208,8 +209,7 @@ def test_post_retry_logging(log_stream):
 UMAPI timeout...service unavailable (code 429 on try 2)
 Next retry in 3 seconds...
 UMAPI timeout...service unavailable (code 429 on try 3)
-Next retry in 3 seconds...
-UMAPI timeout...giving up after 3 attempts.
+UMAPI timeout...giving up after 3 attempts (6 seconds).
 """
 
 

tests/test_functional.py

@@ -59,7 +59,8 @@ def test_user_federatedid_username():
 def test_create_user_adobeid():
     user = UserAction(email="[email protected]")
     user.create()
-    assert user.wire_dict() == {"do": [{"addAdobeID": {"email": "[email protected]"}}],
+    assert user.wire_dict() == {"do": [{"addAdobeID": {"email": "[email protected]",
+                                                       "option": "ignoreIfAlreadyExists"}}],
                                 "user": "[email protected]",
                                 "useAdobeID": True}
 
@@ -75,7 +76,8 @@ def test_create_user_enterpriseid():
     user.create(first_name="Daniel", last_name="Brotsky")
     assert user.wire_dict() == {"do": [{"createEnterpriseID": {"email": "[email protected]",
                                                                "firstname": "Daniel", "lastname": "Brotsky",
-                                                               "country": "UD"}}],
+                                                               "country": "UD",
+                                                               "option": "ignoreIfAlreadyExists"}}],
                                 "user": "[email protected]"}
 
 
@@ -84,7 +86,8 @@ def test_create_user_federatedid():
     user.create(first_name="Daniel", last_name="Brotsky", country="US")
     assert user.wire_dict() == {"do": [{"createFederatedID": {"email": "[email protected]",
                                                               "firstname": "Daniel", "lastname": "Brotsky",
-                                                              "country": "US"}}],
+                                                              "country": "US",
+                                                              "option": "ignoreIfAlreadyExists"}}],
                                 "user": "[email protected]"}
 
 
@@ -93,7 +96,8 @@ def test_create_user_federatedid_username():
     user.create(first_name="Daniel", last_name="Brotsky", country="US", email="[email protected]")
     assert user.wire_dict() == {"do": [{"createFederatedID": {"email": "[email protected]",
                                                               "firstname": "Daniel", "lastname": "Brotsky",
-                                                              "country": "US"}}],
+                                                              "country": "US",
+                                                              "option": "ignoreIfAlreadyExists"}}],
                                 "user": "dbrotsky", "domain": "k.on-the-side.net"}
 
 
@@ -103,7 +107,8 @@ def test_create_user_federatedid_username_email():
     user.create(first_name="Daniel", last_name="Brotsky", country="US")
     assert user.wire_dict() == {"do": [{"createFederatedID": {"email": "[email protected]",
                                                               "firstname": "Daniel", "lastname": "Brotsky",
-                                                              "country": "US"}}],
+                                                              "country": "US",
+                                                              "option": "ignoreIfAlreadyExists"}}],
                                 "user": "dbrotsky", "domain": "k.on-the-side.net"}
 
 

umapi_client/connection.py

@@ -51,7 +51,7 @@ def __init__(self,
                  retry_max_attempts=4,
                  retry_first_delay=15,
                  retry_random_delay=5,
-                 timeout_seconds=60.0,
+                 timeout_seconds=120.0,
                  throttle_actions=10,
                  throttle_commands=10,
                  user_agent=None,
@@ -369,42 +369,49 @@ def call():
             def call():
                 return self.session.get(self.endpoint + path, auth=self.auth, timeout=self.timeout)
 
-        total_time = wait_time = 0
+        total_time = 0
         result = None
         for num_attempts in range(1, self.retry_max_attempts + 1):
-            if wait_time > 0:
-                sleep(wait_time)
-                total_time += wait_time
-                wait_time = 0
             try:
                 result = call()
+                if result.status_code == 200:
+                    return result
+                elif result.status_code in [429, 502, 503, 504]:
+                    if self.logger: self.logger.warning("UMAPI timeout...service unavailable (code %d on try %d)",
+                                                        result.status_code, num_attempts)
+                    retry_wait = 0
+                    if "Retry-After" in result.headers:
+                        advice = result.headers["Retry-After"]
+                        advised_time = parsedate_tz(advice)
+                        if advised_time is not None:
+                            # header contains date
+                            retry_wait = int(mktime_tz(advised_time) - time())
+                        else:
+                            # header contains delta seconds
+                            retry_wait = int(advice)
+                    if retry_wait <= 0:
+                        # use exponential back-off with random delay
+                        delay = randint(0, self.retry_random_delay)
+                        retry_wait = (int(pow(2, num_attempts - 1)) * self.retry_first_delay) + delay
+                elif 201 <= result.status_code < 400:
+                    raise ClientError("Unexpected HTTP Status {:d}: {}".format(result.status_code, result.text), result)
+                elif 400 <= result.status_code < 500:
+                    raise RequestError(result)
+                else:
+                    raise ServerError(result)
             except requests.Timeout:
                 total_time += int(self.timeout)
-                raise UnavailableError(num_attempts, total_time, result)
-            if result.status_code == 200:
-                return result
-            elif result.status_code in [429, 502, 503, 504]:
-                if self.logger: self.logger.warning("UMAPI timeout...service unavailable (code %d on try %d)",
-                                                    result.status_code, num_attempts)
-                if "Retry-After" in result.headers:
-                    advice = result.headers["Retry-After"]
-                    advised_time = parsedate_tz(advice)
-                    if advised_time is not None:
-                        # header contains date
-                        wait_time = int(mktime_tz(advised_time) - time())
-                    else:
-                        # header contains delta seconds
-                        wait_time = int(advice)
-                if wait_time <= 0:
-                    # use exponential back-off with random delay
-                    delay = randint(0, self.retry_random_delay)
-                    wait_time = (int(pow(2, num_attempts)) * self.retry_first_delay) + delay
-                if self.logger: self.logger.warning("Next retry in %d seconds...", wait_time)
-            elif 201 <= result.status_code < 400:
-                raise ClientError("Unexpected HTTP Status {:d}: {}".format(result.status_code, result.text), result)
-            elif 400 <= result.status_code < 500:
-                raise RequestError(result)
-            else:
-                raise ServerError(result)
-        if self.logger: self.logger.error("UMAPI timeout...giving up after %d attempts.", self.retry_max_attempts)
+                if self.logger: self.logger.warning("UMAPI connection timeout...(%d seconds on try %d)",
+                                                    self.timeout, num_attempts)
+                retry_wait = 0
+                result = None
+            if num_attempts < self.retry_max_attempts:
+                if retry_wait > 0:
+                    if self.logger: self.logger.warning("Next retry in %d seconds...", retry_wait)
+                    sleep(retry_wait)
+                    total_time += retry_wait
+                else:
+                    if self.logger: self.logger.warning("Immediate retry...", retry_wait)
+        if self.logger: self.logger.error("UMAPI timeout...giving up after %d attempts (%d seconds).",
+                                          self.retry_max_attempts, total_time)
         raise UnavailableError(self.retry_max_attempts, total_time, result)

umapi_client/functional.py

@@ -127,9 +127,12 @@ def __init__(self, id_type=IdentityTypes.adobeID, email=None, username=None, dom
             Action.__init__(self, user=email, **kwargs)
 
     def create(self, first_name=None, last_name=None, country=None, email=None,
-               on_conflict=IfAlreadyExistsOptions.errorIfAlreadyExists):
+               on_conflict=IfAlreadyExistsOptions.ignoreIfAlreadyExists):
         """
         Create the user on the Adobe back end.
+        See [Issue 32](https://github.com/adobe-apiplatform/umapi-client.py/issues/32): because
+        we retry create calls if they time out, the default conflict handling for creation is to ignore the
+        create call if the user already exists (possibly from an earlier call that timed out).
         :param first_name: (optional) user first name
         :param last_name: (optional) user last name
         :param country: (optional except for Federated ID) user 2-letter ISO country code

umapi_client/version.py

@@ -18,4 +18,4 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-__version__ = "2.0.2"
+__version__ = "2.0.3"