SG-38213 Prevent unexpected retries on error (#379)

* Prevent unexpected retries on error

* Fix tests

* Update shotgun_api3/shotgun.py

Co-authored-by: Julien Langlois <[email protected]>

* Packaging for v3.8.3-beta1

* Fix typo

* Update HISTORY.rst

Co-authored-by: Julien Langlois <[email protected]>

---------

Co-authored-by: Julien Langlois <[email protected]>

Author: carlos-villavicencio-adsk

HISTORY.rst

@@ -4,6 +4,13 @@ Flow Production Tracking Python API Changelog
 
 Here you can see the full list of changes between each Python API release.
 
+v3.8.5 (2025 Xxx X)
+===================
+
+- We don't want to retry on general exceptions (e.g. timeout or remote disconnection)
+  because we might send a resource modification request (create, batch create, etc) and
+  we can end up duplicating things.
+
 v3.8.4 (2025 Jun 11)
 ====================
 

shotgun_api3/shotgun.py

@@ -3938,11 +3938,10 @@ def _make_call(self, verb, path, body, headers):
                 if attempt == max_rpc_attempts:
                     LOG.debug("Request failed.  Giving up after %d attempts." % attempt)
                     raise
-            except Exception:
+            except Exception as e:
                 self._close_connection()
-                if attempt == max_rpc_attempts:
-                    LOG.debug("Request failed.  Giving up after %d attempts." % attempt)
-                    raise
+                LOG.debug(f"Request failed.  Reason: {e}", exc_info=True)
+                raise
 
             LOG.debug(
                 "Request failed, attempt %d of %d.  Retrying in %.2f seconds..."

tests/test_api.py

@@ -2223,42 +2223,8 @@ def test_make_call_retry(self, mock_request):
 
             self.assertEqual(cm2.exception.args[0], "not working")
             log_content = "\n".join(cm1.output)
-            for i in [1, 2]:
-                self.assertIn(
-                    f"Request failed, attempt {i} of 3.  Retrying",
-                    log_content,
-                )
-            self.assertIn(
-                "Request failed.  Giving up after 3 attempts.",
-                log_content,
-            )
-
-            # Then, make the exception happening only once and prove the
-            # retry works
-            def my_side_effect(*args, **kwargs):
-                try:
-                    if my_side_effect.counter < 1:
-                        raise Exception("not working")
-
-                    return mock.DEFAULT
-                finally:
-                    my_side_effect.counter += 1
-
-            my_side_effect.counter = 0
-            mock_request.side_effect = my_side_effect
-            with self.assertLogs("shotgun_api3", level="DEBUG") as cm:
-                self.assertIsInstance(
-                    self.sg.info(),
-                    dict,
-                )
-
-            log_content = "\n".join(cm.output)
             self.assertIn(
-                "Request failed, attempt 1 of 3.  Retrying",
-                log_content,
-            )
-            self.assertNotIn(
-                "Request failed, attempt 2 of 3.  Retrying",
+                "Request failed.  Reason: not working",
                 log_content,
             )
 

tests/test_client.py

@@ -330,17 +330,9 @@ def test_network_retry(self):
         with mock.patch("time.sleep") as mock_sleep:
             self.assertRaises(httplib2.HttpLib2Error, self.sg.info)
             self.assertTrue(
-                self.sg.config.max_rpc_attempts == self.sg._http_request.call_count,
+                self.sg._http_request.call_count == 1,
                 "Call is repeated",
             )
-            # Ensure that sleep was called with the retry interval between each attempt
-            attempt_interval = self.sg.config.rpc_attempt_interval / 1000.0
-            calls = [mock.callargs(((attempt_interval,), {}))]
-            calls *= self.sg.config.max_rpc_attempts - 1
-            self.assertTrue(
-                mock_sleep.call_args_list == calls,
-                "Call is repeated at correct interval.",
-            )
 
     def test_set_retry_interval(self):
         """Setting the retry interval through parameter and environment variable works."""