Add default 3600s expiry for vCons created via POST endpoints #120

howethomas · 2026-01-26T23:16:16Z

Refactor VconRedis class with TTL support:
- Add optional ttl parameter to store_vcon() and store_vcon_dict()
- Add set_expiry(), get_ttl(), remove_expiry() methods
- Add async variants for all TTL operations
- Add DEFAULT_TTL class attribute from VCON_REDIS_EXPIRY setting
Update POST /vcon endpoint to set VCON_REDIS_EXPIRY after storing
Update POST /vcon/external-ingress endpoint with same expiry behavior
Add comprehensive unit tests:
- tests/core/test_vcon_redis_ttl.py (24 tests for VconRedis TTL)
- server/tests/test_post_vcon_expiry.py (8 tests for API expiry)
Update documentation:
- Clarify VCON_REDIS_EXPIRY applies to POST-created vCons
- Document cache expiry behavior in API reference

- Refactor VconRedis class with TTL support: - Add optional ttl parameter to store_vcon() and store_vcon_dict() - Add set_expiry(), get_ttl(), remove_expiry() methods - Add async variants for all TTL operations - Add DEFAULT_TTL class attribute from VCON_REDIS_EXPIRY setting - Update POST /vcon endpoint to set VCON_REDIS_EXPIRY after storing - Update POST /vcon/external-ingress endpoint with same expiry behavior - Add comprehensive unit tests: - tests/core/test_vcon_redis_ttl.py (24 tests for VconRedis TTL) - server/tests/test_post_vcon_expiry.py (8 tests for API expiry) - Update documentation: - Clarify VCON_REDIS_EXPIRY applies to POST-created vCons - Document cache expiry behavior in API reference

Copilot

Pull request overview

This PR adds default TTL (Time-To-Live) expiry functionality for vCons created via POST endpoints. The implementation includes refactoring the VconRedis class to support optional TTL parameters, updating API endpoints to set expiry on newly created vCons, comprehensive test coverage, and updated documentation.

Changes:

Refactored VconRedis class with TTL support including optional ttl parameters for store methods and new TTL management methods (set_expiry, get_ttl, remove_expiry)
Updated POST /vcon and POST /vcon/external-ingress endpoints to automatically set VCON_REDIS_EXPIRY (default 3600s) on stored vCons
Added 24 unit tests for VconRedis TTL functionality and 8 tests for API endpoint expiry behavior

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
server/lib/vcon_redis.py	Adds TTL support to VconRedis class with optional ttl parameters for store methods, new TTL management methods, async variants, and DEFAULT_TTL class attribute
server/api.py	Updates POST /vcon and POST /vcon/external-ingress endpoints to set default VCON_REDIS_EXPIRY TTL on newly created vCons
tests/core/test_vcon_redis_ttl.py	Comprehensive unit tests (24 tests) for VconRedis TTL functionality including sync/async variants and integration tests
server/tests/test_post_vcon_expiry.py	Unit tests (8 tests) verifying POST endpoints set default expiry correctly
docs/operations/api-reference.md	Documents cache expiry behavior for POST /vcon and POST /vcon/external-ingress endpoints
docs/configuration/environment-variables.md	Updates VCON_REDIS_EXPIRY documentation to clarify it applies to POST-created vCons

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-26T23:22:43Z

tests/core/test_vcon_redis_ttl.py

+class TestAsyncStoreVconDictWithTTL:
+    """Tests for async store_vcon_dict_async method."""
+
+    @pytest.mark.asyncio
+    async def test_store_vcon_dict_async_with_ttl(self, vcon_redis, sample_vcon_dict):
+        """Verify store_vcon_dict_async with TTL sets the expiry."""
+        mock_redis_async = MagicMock()
+        mock_json = MagicMock()
+        mock_json.set = AsyncMock()
+        mock_redis_async.json.return_value = mock_json
+        mock_redis_async.expire = AsyncMock()
+
+        custom_ttl = 1800
+        await vcon_redis.store_vcon_dict_async(mock_redis_async, sample_vcon_dict, ttl=custom_ttl)
+
+        expected_key = f"vcon:{sample_vcon_dict['uuid']}"
+        mock_redis_async.expire.assert_called_once_with(expected_key, custom_ttl)
+


Test coverage is incomplete for the store_vcon_dict_async method. Add a test case to verify that calling store_vcon_dict_async without the TTL parameter does not set expiry, similar to the existing test_store_vcon_async_without_ttl test.

Copilot · 2026-01-26T23:22:43Z

tests/core/test_vcon_redis_ttl.py

+class TestAsyncGetTTL:
+    """Tests for async get_ttl_async method."""
+
+    @pytest.mark.asyncio
+    async def test_get_ttl_async_with_expiry(self, vcon_redis, sample_vcon_dict):
+        """Verify get_ttl_async returns remaining TTL when set."""
+        mock_redis_async = MagicMock()
+        mock_redis_async.ttl = AsyncMock(return_value=1800)
+
+        result = await vcon_redis.get_ttl_async(mock_redis_async, sample_vcon_dict['uuid'])
+
+        assert result == 1800
+
+    @pytest.mark.asyncio
+    async def test_get_ttl_async_no_expiry(self, vcon_redis, sample_vcon_dict):
+        """Verify get_ttl_async returns -1 when no expiry is set."""
+        mock_redis_async = MagicMock()
+        mock_redis_async.ttl = AsyncMock(return_value=-1)
+
+        result = await vcon_redis.get_ttl_async(mock_redis_async, sample_vcon_dict['uuid'])
+
+        assert result == -1
+


Test coverage is incomplete for the get_ttl_async method. Add a test case to verify that it returns -2 when the key doesn't exist, similar to the existing test_get_ttl_key_not_found test for the synchronous version.

Copilot · 2026-01-26T23:22:44Z

server/lib/vcon_redis.py

+                 -2 if the key doesn't exist.
+        """
+        key = f"vcon:{vcon_id}"
+        return await redis_async.ttl(key)


The PR description states "Add async variants for all TTL operations", but the async variant for remove_expiry is missing. For consistency and completeness, add a remove_expiry_async method that mirrors the synchronous remove_expiry method.

Suggested change

return await redis_async.ttl(key)

return await redis_async.ttl(key)

async def remove_expiry_async(self, redis_async, vcon_id: str) -> bool:

"""Asynchronously removes the expiry from a vCon, making it persistent.

Args:

redis_async: Async Redis client instance.

vcon_id (str): The vCon UUID.

Returns:

bool: True if the expiry was removed or the key is now persistent,

False if the key doesn't exist or had no expiry.

"""

key = f"vcon:{vcon_id}"

result = await redis_async.persist(key)

if result:

logger.debug(f"Removed expiry from vCon {vcon_id}")

return bool(result)

Copilot · 2026-01-26T23:22:44Z

tests/core/test_vcon_redis_ttl.py

+import json
+import os
+import pytest
+import asyncio


The asyncio module is imported but never used in this test file. Remove this unused import to keep the code clean.

Suggested change

import asyncio

Copilot · 2026-01-26T23:22:44Z

server/tests/test_post_vcon_expiry.py

+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest


Import of 'pytest' is not used.

Suggested change

import pytest

Copilot · 2026-01-26T23:22:45Z

tests/core/test_vcon_redis_ttl.py

+        try:
+            from redis_mgr import redis
+            redis.delete(f"vcon:{sample_vcon_obj.uuid}")
+        except Exception:


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except Exception:

except Exception:

# Ignore cleanup errors during teardown to avoid masking test failures

The existing tests were failing because they didn't mock redis_async.expire() which was added as part of the default TTL feature.

slyapustin

@howethomas It makes sense as we eventually get stuck with old, broken vCons in Redis.

The change that introduced a default expiry configuration will affect the DLQ flow, since those vCons may expire before you attempt to reprocess them.

I would suggest either changing the default behavior so they do not expire, while keeping the configuration available for anyone who wants it, or reviewing how the DLQ is implemented so it increases the TTL for vCons stored there.

When a vCon fails processing and is moved to the Dead Letter Queue, its TTL is now extended to VCON_DLQ_EXPIRY (default 7 days) to ensure operators have time to investigate before the vCon expires from Redis. - Add VCON_DLQ_EXPIRY setting (default 604800 seconds / 7 days) - Set to 0 to disable DLQ expiry extension - Update worker_loop to extend vCon TTL when moving to DLQ - Add unit tests for DLQ expiry behavior - Update documentation for environment variables and DLQ operations

slyapustin

Looks good

howethomas requested review from Copilot and slyapustin January 26, 2026 23:16

Copilot started reviewing on behalf of howethomas January 26, 2026 23:17 View session

Copilot AI reviewed Jan 26, 2026

View reviewed changes

Fix CI: add expire mock to existing external ingress tests

869cac7

The existing tests were failing because they didn't mock redis_async.expire() which was added as part of the default TTL feature.

slyapustin requested changes Jan 27, 2026

View reviewed changes

slyapustin approved these changes Jan 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add default 3600s expiry for vCons created via POST endpoints #120

Add default 3600s expiry for vCons created via POST endpoints #120

howethomas commented Jan 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

slyapustin left a comment

Uh oh!

slyapustin left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-        return await redis_async.ttl(key)
+        return await redis_async.ttl(key)
+    async def remove_expiry_async(self, redis_async, vcon_id: str) -> bool:
+        """Asynchronously removes the expiry from a vCon, making it persistent.
+        Args:
+            redis_async: Async Redis client instance.
+            vcon_id (str): The vCon UUID.
+        Returns:
+            bool: True if the expiry was removed or the key is now persistent,
+                  False if the key doesn't exist or had no expiry.
+        """
+        key = f"vcon:{vcon_id}"
+        result = await redis_async.persist(key)
+        if result:
+            logger.debug(f"Removed expiry from vCon {vcon_id}")
+        return bool(result)


		from unittest.mock import AsyncMock, MagicMock, patch

		import pytest

	except Exception:
	except Exception:
	# Ignore cleanup errors during teardown to avoid masking test failures

Add default 3600s expiry for vCons created via POST endpoints #120

Are you sure you want to change the base?

Add default 3600s expiry for vCons created via POST endpoints #120

Conversation

howethomas commented Jan 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

slyapustin left a comment

Choose a reason for hiding this comment

Uh oh!

slyapustin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants