Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion products/feature_flags/backend/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
from posthog.api.capture import capture_batch_internal
from posthog.models.team import Team
from posthog.scoping_audit import skip_team_scope_audit
from posthog.storage.hypercache_manager import HYPERCACHE_SIGNAL_UPDATE_COUNTER
from posthog.storage.hypercache_manager import (
HYPERCACHE_SIGNAL_UPDATE_COUNTER,
get_cache_stats as get_cache_stats_generic,
)
from posthog.tasks.utils import CeleryQueue, PushGatewayTask

from products.feature_flags.backend.canary import run_local_eval_canary
Expand Down Expand Up @@ -489,11 +492,18 @@ def refresh_expiring_flag_definitions_cache_entries(self: PushGatewayTask) -> No
successful_gauge.set(counts.successful)
failed_gauge.set(counts.failed)

# Scan after refresh for metrics (pushes to Pushgateway via get_cache_stats)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Comment narrates the code

This comment only restates that the next call scans and pushes cache statistics. The repository requires comments to explain non-obvious rationale rather than narrate the code, so this requirement must be satisfied before merging.

Context Used: AGENTS.MD (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: products/feature_flags/backend/tasks.py
Line: 495

Comment:
**Comment narrates the code**

This comment only restates that the next call scans and pushes cache statistics. The repository requires comments to explain non-obvious rationale rather than narrate the code, so this requirement must be satisfied before merging.

**Context Used:** AGENTS.MD ([source](https://github.com/posthog/posthog/blob/master/AGENTS.MD))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

stats_after = get_cache_stats_generic(FLAG_DEFINITIONS_HYPERCACHE_MANAGEMENT_CONFIG)

duration = time.time() - start_time
logger.info(
"Completed flag definitions cache refresh",
successful_refreshes=counts.successful,
failed_refreshes=counts.failed,
total_cached=stats_after.get("total_cached", 0),
total_teams=stats_after.get("total_teams", 0),
cache_coverage=stats_after.get("cache_coverage", "unknown"),
ttl_distribution=stats_after.get("ttl_distribution", {}),
duration_seconds=duration,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,25 @@
from posthog.storage.cache_expiry_manager import CacheRefreshCounts
from posthog.tasks.test.utils import PushGatewayTaskTestMixin

from products.feature_flags.backend.local_evaluation import FLAG_DEFINITIONS_HYPERCACHE_MANAGEMENT_CONFIG
from products.feature_flags.backend.tasks import (
cleanup_stale_flag_definitions_expiry_tracking_task,
refresh_expiring_flag_definitions_cache_entries,
)


class TestRefreshExpiringFlagDefinitionsCacheEntries(PushGatewayTaskTestMixin, TestCase):
@patch("products.feature_flags.backend.tasks.get_cache_stats_generic", return_value={})
@patch("posthog.storage.cache_expiry_manager.refresh_expiring_caches")
def test_refreshes_cache(self, mock_refresh: MagicMock) -> None:
def test_refreshes_cache(self, mock_refresh: MagicMock, mock_stats: MagicMock) -> None:
mock_refresh.return_value = CacheRefreshCounts(successful=5, failed=0)

refresh_expiring_flag_definitions_cache_entries()

mock_refresh.assert_called_once()
assert self.registry.get_sample_value("posthog_flag_definitions_cache_refresh_successful_count") == 5
assert self.registry.get_sample_value("posthog_flag_definitions_cache_refresh_failed_count") == 0
mock_stats.assert_called_once_with(FLAG_DEFINITIONS_HYPERCACHE_MANAGEMENT_CONFIG)

@patch("posthog.storage.cache_expiry_manager.refresh_expiring_caches")
def test_propagates_error(self, mock_refresh: MagicMock) -> None:
Expand Down
Loading