unstract/sdk1/src/unstract/sdk1/utils/metrics_mixin.py (1)

54-60: Avoid exists() + get() race and extra Redis roundtrip.

exists() followed by get() can fail if the key expires/deletes between calls; reading once and handling None is simpler and quieter.

♻️ Proposed change

-            if not self.redis_client.exists(self.redis_key):
-                return {self.TIME_TAKEN_KEY: None}
-
-            start_time = float(self.redis_client.get(self.redis_key))
-            time_taken = round(time.time() - start_time, 3)
-
-            self.redis_client.delete(self.redis_key)
+            start_time_raw = self.redis_client.get(self.redis_key)
+            if start_time_raw is None:
+                return {self.TIME_TAKEN_KEY: None}
+
+            time_taken = round(time.time() - float(start_time_raw), 3)
+            self.redis_client.delete(self.redis_key)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@unstract/sdk1/src/unstract/sdk1/utils/metrics_mixin.py` around lines 54 - 60,
Replace the exists()+get() pattern with a single read and a None check: call
self.redis_client.get(self.redis_key) once, return {self.TIME_TAKEN_KEY: None}
if the result is None, otherwise convert the value to float, compute time_taken
= round(time.time() - start_time, 3), then delete the key with
self.redis_client.delete(self.redis_key); reference the existing symbols
self.redis_client, self.redis_key, and self.TIME_TAKEN_KEY when making this
change to avoid the race and extra Redis roundtrip.

unstract/core/src/unstract/core/cache/redis_client.py (1)

300-308: Consider adding an __init__ method for safer instantiation.

The class declares redis_client: redis.Redis as a type hint without a default, and relies on from_env() or manual __new__ + attribute assignment. This works but is fragile—calling methods on an improperly constructed instance will raise AttributeError.

Adding an explicit __init__ would make the expected construction pattern clearer and prevent misuse.

💡 Optional: Add __init__ for explicit construction

 class RedisClient:
     """Wrapper around redis.Redis providing a consistent interface.

     Always instantiate via RedisClient.from_env(), which delegates to
     create_redis_client() and handles both Sentinel and standalone modes.
     """

-    redis_client: redis.Redis
+    def __init__(self, redis_client: redis.Redis):
+        """Initialize with an existing redis.Redis client.
+        
+        Prefer using from_env() for environment-based construction.
+        """
+        self.redis_client = redis_client

     # ... existing methods ...

     `@classmethod`
     def from_env(cls, env_prefix: str = "REDIS_") -> "RedisClient":
         """Create client from environment variables."""
-        instance = cls.__new__(cls)
-        instance.redis_client = create_redis_client(
+        return cls(create_redis_client(
             env_prefix=env_prefix, decode_responses=True
-        )
-        return instance
+        ))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@unstract/core/src/unstract/core/cache/redis_client.py` around lines 300 -
308, Add an explicit __init__ to RedisClient that accepts a redis_client:
redis.Redis (or Optional[redis.Redis] with validation) and assigns it to
self.redis_client, and raise a clear ValueError if None or not an instance of
redis.Redis; update the class docstring to note the preferred construction via
RedisClient.from_env() or RedisClient(redis_client=...) so callers don't
accidentally create instances without the required attribute. Ensure references
to create_redis_client() and from_env() remain compatible with the new
constructor.

workers/shared/cache/cache_backends.py (1)

102-111: Consider using RedisClient.from_env() or a proper constructor.

The manual __new__ + attribute assignment pattern works but bypasses any future __init__ logic. Since this code needs custom connection parameters that from_env() doesn't support, consider either:

1. Extending from_env() to accept additional kwargs, or
2. Adding a proper __init__ to RedisClient (as suggested in the other file)

For now, adding a comment explaining why this pattern is necessary would help maintainability.

💡 Option 1: If RedisClient gets an __init__, use it directly

             raw_client = create_redis_client(
                 env_prefix="CACHE_REDIS_",
                 decode_responses=True,
                 socket_timeout=5,
                 socket_connect_timeout=5,
                 health_check_interval=30,
             )
-            # Wrap in RedisClient for interface compatibility
-            self.redis_client = RedisClient.__new__(RedisClient)
-            self.redis_client.redis_client = raw_client
+            # Wrap in RedisClient for interface compatibility
+            self.redis_client = RedisClient(raw_client)

💡 Option 2: If keeping __new__ pattern, add explanatory comment

             raw_client = create_redis_client(
                 env_prefix="CACHE_REDIS_",
                 decode_responses=True,
                 socket_timeout=5,
                 socket_connect_timeout=5,
                 health_check_interval=30,
             )
-            # Wrap in RedisClient for interface compatibility
+            # Wrap raw client in RedisClient for interface compatibility.
+            # Using __new__ because RedisClient has no __init__ and from_env()
+            # doesn't support custom connection parameters.
             self.redis_client = RedisClient.__new__(RedisClient)
             self.redis_client.redis_client = raw_client

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@workers/shared/cache/cache_backends.py` around lines 102 - 111, The current
code instantiates RedisClient via RedisClient.__new__ and assigns redis_client
directly after create_redis_client; update this to either extend
RedisClient.from_env to accept the custom kwargs from create_redis_client or add
a proper RedisClient.__init__ that accepts (and uses)
decode_responses/socket_timeout/socket_connect_timeout/health_check_interval and
call that instead of using __new__; if you choose to keep the __new__ pattern
for now, add a clear explanatory comment above the RedisClient.__new__ usage
referencing create_redis_client and why __init__ is bypassed (mention
RedisClient, RedisClient.from_env, and the redis_client attribute) so future
maintainers know this is intentional and should be revisited when RedisClient
gains an __init__ or extended from_env.