-
Notifications
You must be signed in to change notification settings - Fork 41
feat: SSE support in provider #975
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import asyncio | ||
| import logging | ||
| import os | ||
| import sys | ||
|
|
||
| _PYTHON_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) | ||
| sys.path.insert(0, os.path.join(_PYTHON_DIR, "sdk")) | ||
| sys.path.insert(0, os.path.join(_PYTHON_DIR, "bindings")) | ||
|
|
||
| from openfeature.evaluation_context import EvaluationContext | ||
| from superposition_provider import LocalResolutionProvider, HttpDataSource | ||
| from superposition_provider.types import SuperpositionOptions, SseStrategy | ||
|
|
||
| logging.basicConfig(level=logging.INFO, format="%(asctime)s %(message)s") | ||
| logging.getLogger("superposition_sdk").setLevel(logging.CRITICAL) | ||
| logging.getLogger("superposition_provider").setLevel(logging.WARNING) | ||
| log = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| ENDPOINT = os.environ.get("SUPERPOSITION_ENDPOINT", "http://localhost:8080") | ||
| TOKEN = os.environ.get("SUPERPOSITION_TOKEN", "token") | ||
| ORG = os.environ.get("SUPERPOSITION_ORG_ID", "localorg") | ||
| WORKSPACE = os.environ.get("SUPERPOSITION_WORKSPACE", "dev") | ||
|
|
||
|
|
||
| async def main(): | ||
| loop = asyncio.get_event_loop() | ||
| _orig = loop.default_exception_handler | ||
| loop.set_exception_handler( | ||
| lambda l, ctx: None if ctx.get("message") == "Unclosed client session" else _orig(ctx) | ||
| ) | ||
|
|
||
| options = SuperpositionOptions(endpoint=ENDPOINT, token=TOKEN, org_id=ORG, workspace_id=WORKSPACE) | ||
|
|
||
| def on_config_change(before, after): | ||
| for key, value in after.items(): | ||
| if before.get(key) != value: | ||
| log.info(f"[UPDATE] {key}: {before.get(key)!r} -> {value!r}") | ||
|
|
||
| provider = LocalResolutionProvider( | ||
| primary_source=HttpDataSource(options), | ||
| refresh_strategy=SseStrategy(superposition_options=options, reconnect_delay=5), | ||
| on_config_change=on_config_change, | ||
| ) | ||
|
|
||
| await provider.initialize(EvaluationContext()) | ||
| log.info(f"Initial config: {provider.resolve_all_features(EvaluationContext())}") | ||
| log.info("Listening for SSE config changes (Ctrl-C to stop)") | ||
|
|
||
| try: | ||
| await asyncio.Event().wait() | ||
| except KeyboardInterrupt: | ||
| pass | ||
| finally: | ||
| await provider.shutdown() | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| asyncio.run(main()) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -197,6 +197,13 @@ def supports_experiments(self) -> bool: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def close(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Close the HTTP client.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Close the HTTP client and its underlying aiohttp session.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if self.client: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| http_client = getattr(self.client._config, "http_client", None) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| session = getattr(http_client, "_session", None) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if session and not session.closed: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await session.close() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
201
to
+208
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if self.client: | |
| try: | |
| http_client = getattr(self.client._config, "http_client", None) | |
| session = getattr(http_client, "_session", None) | |
| if session and not session.closed: | |
| await session.close() | |
| except Exception: | |
| pass | |
| if not self.client: | |
| return | |
| client = self.client | |
| try: | |
| close_method = getattr(client, "close", None) | |
| if callable(close_method): | |
| close_result = close_method() | |
| if hasattr(close_result, "__await__"): | |
| await close_result | |
| return | |
| http_client = getattr(getattr(client, "_config", None), "http_client", None) | |
| session = getattr(http_client, "_session", None) | |
| if session and not session.closed: | |
| await session.close() | |
| except Exception: | |
| logger.warning("Failed to close Superposition HTTP client cleanly", exc_info=True) | |
| finally: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reaching into private SDK internals + silent exception swallow.
self.client._config.http_client._session traverses two layers of underscore-prefixed attributes of the SDK/HTTP client. Any internal rename in superposition_sdk or its HTTP backend will silently stop closing the aiohttp session (re-introducing the "Unclosed client session" warnings this change is meant to fix), and the broad except Exception: pass hides it. Consider:
- Exposing a public
close()/aclose()on the SDK client (or its HTTP client) and calling that instead. - At minimum, logging the exception at
debug/warningso regressions are observable (ruff S110/BLE001).
Proposed logging tweak
- try:
- http_client = getattr(self.client._config, "http_client", None)
- session = getattr(http_client, "_session", None)
- if session and not session.closed:
- await session.close()
- except Exception:
- pass
+ try:
+ http_client = getattr(self.client._config, "http_client", None)
+ session = getattr(http_client, "_session", None)
+ if session and not session.closed:
+ await session.close()
+ except Exception as e: # noqa: BLE001
+ logger.debug("Failed to close underlying aiohttp session: %s", e)🧰 Tools
🪛 Ruff (0.15.10)
[error] 229-230: try-except-pass detected, consider logging the exception
(S110)
[warning] 229-229: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/python/provider/superposition_provider/http_data_source.py` around
lines 221 - 231, The close method should not reach into private internals or
silently swallow errors: in SuperpositionProvider.close, first check for and
call a public close/aclose on self.client (e.g., if hasattr(self.client,
"aclose"): await self.client.aclose() elif hasattr(self.client, "close"): await
maybe_awaitable(self.client.close())), and only if no public close exists fall
back to the current getattr chain (client._config -> http_client -> _session)
but catch exceptions and log them at debug/warning instead of pass; update
references to self.client, client._config, http_client and _session accordingly
and ensure you do not suppress the exception silently. (Implement
maybe_awaitable as appropriate or await when coroutine.)
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,7 +10,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import weakref | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from datetime import datetime, timezone | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import Dict, List, Optional, Any, Tuple, Union, Sequence, Mapping | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import Callable, Dict, List, Optional, Any, Tuple, Union, Sequence, Mapping | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from openfeature.provider import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AbstractProvider, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -26,7 +26,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from . import FetchResponse | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from .data_source import SuperpositionDataSource, ConfigData, ExperimentData | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from .interfaces import AllFeatureProvider, FeatureExperimentMeta | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from .types import RefreshStrategy, OnDemandStrategy, WatchStrategy, PollingStrategy, ManualStrategy, default_on_demand_strategy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from .types import RefreshStrategy, OnDemandStrategy, WatchStrategy, PollingStrategy, ManualStrategy, SseStrategy, default_on_demand_strategy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -45,6 +45,7 @@ def __init__( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| primary_source: SuperpositionDataSource, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fallback_source: Optional[SuperpositionDataSource] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refresh_strategy: RefreshStrategy = default_on_demand_strategy(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| on_config_change: Optional[Callable[[Dict[str, Any], Dict[str, Any]], None]] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Initialize local resolution provider. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -56,6 +57,7 @@ def __init__( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.primary_source = primary_source | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.fallback_source = fallback_source | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.refresh_strategy = refresh_strategy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.on_config_change = on_config_change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.metadata = Metadata(name="LocalResolutionProvider") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.status = ProviderStatus.NOT_READY | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -68,6 +70,8 @@ def __init__( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Background task for refresh strategy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self._background_task: Optional[asyncio.Task] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Set once the SSE connection is established (SseStrategy only) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self._sse_connected_event: Optional[asyncio.Event] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def initialize(self, context: EvaluationContext): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Initialize the provider. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -358,7 +362,21 @@ async def refresh(self) -> None: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Useful for MANUAL refresh strategy. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await asyncio.gather(self._fetch_and_cache_config(), self._fetch_and_cache_experiments()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| before = self.resolve_all_features(EvaluationContext()) if self.on_config_change and self.ffi_cache else None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results = await asyncio.gather( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self._fetch_and_cache_config(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self._fetch_and_cache_experiments(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return_exceptions=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for result in results: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if isinstance(result, Exception): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.warning(f"Error during refresh: {result}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+371
to
+375
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | |
| for result in results: | |
| if isinstance(result, Exception): | |
| logger.warning(f"Error during refresh: {result}") | |
| ) | |
| errors = [] | |
| for result in results: | |
| if isinstance(result, Exception): | |
| logger.warning(f"Error during refresh: {result}") | |
| errors.append(result) | |
| if errors: | |
| raise errors[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm where resolve_all_features is defined (likely interfaces.py AllFeatureProvider).
fd -e py . clients/python/provider | xargs rg -nP '\bdef\s+resolve_all_features\b' -C2Repository: juspay/superposition
Length of output: 482
🏁 Script executed:
# Check LocalProvider class definition and inheritance
rg -n 'class LocalProvider' clients/python/provider -A 5Repository: juspay/superposition
Length of output: 693
🏁 Script executed:
# Check for _sse_connected_event usage
rg -n '_sse_connected_event' clients/python/providerRepository: juspay/superposition
Length of output: 326
🏁 Script executed:
# Find options/org_id dataclass definition
rg -n 'org_id' clients/python/provider/superposition_provider/local_provider.pyRepository: juspay/superposition
Length of output: 278
🏁 Script executed:
# Review SSE parsing logic - search for SSE event handling
rg -n 'event:|data:' clients/python/provider -A 2 -B 2Repository: juspay/superposition
Length of output: 6681
🏁 Script executed:
# Get complete LocalProviderOptions definition
rg -n 'class LocalProviderOptions' clients/python/provider -A 20Repository: juspay/superposition
Length of output: 1951
🏁 Script executed:
# Check if _sse_connected_event is ever awaited or used
rg -n 'await.*_sse_connected_event|_sse_connected_event.wait|_sse_connected_event.set' clients/python/providerRepository: juspay/superposition
Length of output: 46
🏁 Script executed:
# Find LocalProvider class definition and verify inheritance
rg -n 'class LocalProvider' clients/python/provider -A 10Repository: juspay/superposition
Length of output: 1130
🏁 Script executed:
# Search for LocalProvider class definition in local_provider.py
rg -n 'class LocalProvider' clients/python/provider/superposition_provider/local_provider.py -A 15Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
# Get context around lines 510-520 to see what options object is used there
sed -n '510,520p' clients/python/provider/superposition_provider/local_provider.pyRepository: juspay/superposition
Length of output: 740
🏁 Script executed:
# Check if AllFeatureProvider is imported in local_provider.py
rg -n 'AllFeatureProvider|from.*interfaces' clients/python/provider/superposition_provider/local_provider.pyRepository: juspay/superposition
Length of output: 279
🏁 Script executed:
# Get full signature of resolve_all_features
rg -n 'def resolve_all_features' clients/python/provider/superposition_provider/interfaces.py -A 5Repository: juspay/superposition
Length of output: 1040
🏁 Script executed:
# Also verify lines 363-377 match what was shown in review
sed -n '363,377p' clients/python/provider/superposition_provider/local_provider.pyRepository: juspay/superposition
Length of output: 714
Remove the AttributeError concern — resolve_all_features is correctly inherited from AllFeatureProvider.
The method is defined in the AllFeatureProvider interface (signature: context: Optional[EvaluationContext] → Dict[str, Any]) and LocalResolutionProvider inherits from it, so this call is valid.
That said, address these issues:
_sse_connected_eventis a dead field: Set at line 520 but never awaited or used. Remove it.- Redundant org_id check at line 516: Since
org_idis a required field inSuperpositionProviderOptions(no default value),if options.org_id:always evaluates toTrue. Remove the conditional. - SSE parsing at line 553 fires on every
event:ordata:line individually: This violates the Server-Sent Events spec, which defines event boundaries at blank lines. Currently, a single SSE event may trigger multiple refresh calls in rapid succession before being cancelled, causing unnecessary churn. Restructure to accumulate lines until a blank line is encountered.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/python/provider/superposition_provider/local_provider.py` around
lines 363 - 377, Keep the resolve_all_features call as-is (it’s inherited),
remove the dead _sse_connected_event field and any assignments/usages of it, and
delete the redundant conditional around options.org_id
(SuperpositionProviderOptions.org_id is required so the "if options.org_id:"
branch is unnecessary). For the SSE loop (the method that reads the event stream
and currently handles "event:" / "data:" lines one-by-one), change the parser to
accumulate successive lines into a buffer and only treat an empty line as the
end of a single SSE event; on that boundary parse the buffered event/data block
and trigger a single refresh (e.g., call the existing refresh/notification path
once per completed event) rather than firing on every line. Ensure you update
any related cancellation/cleanup logic accordingly so events are debounced by
the SSE boundary.
Copilot
AI
Apr 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SSE client loop iterates async for line_bytes in resp.content, but aiohttp yields arbitrary chunk boundaries, not guaranteed line-delimited SSE frames. This can merge/split event:/data: lines and cause missed or spurious refresh triggers. Use a line-based reader (e.g., await resp.content.readline() in a loop) and parse SSE events based on \n\n frame separation.
| async for line_bytes in resp.content: | |
| self_ref = weak_self() | |
| if self_ref is None: | |
| logger.info("Provider garbage collected, stopping SSE loop.") | |
| return | |
| line = line_bytes.decode("utf-8", errors="replace").strip() | |
| logger.debug(f"SSE raw line: {line!r}") | |
| if not line or line.startswith(":"): | |
| continue | |
| if line.startswith("event:") or line.startswith("data:"): | |
| async def _do_refresh(ref=self_ref): | |
| await asyncio.sleep(debounce_s) | |
| try: | |
| await ref.refresh() | |
| except Exception as e: | |
| logger.warning(f"SSE-triggered refresh failed: {e}") | |
| if debounce_task and not debounce_task.done(): | |
| debounce_task.cancel() | |
| debounce_task = asyncio.create_task(_do_refresh()) | |
| event_lines: List[str] = [] | |
| while True: | |
| line_bytes = await resp.content.readline() | |
| if line_bytes == b"": | |
| break | |
| self_ref = weak_self() | |
| if self_ref is None: | |
| logger.info("Provider garbage collected, stopping SSE loop.") | |
| return | |
| line = line_bytes.decode("utf-8", errors="replace").rstrip("\r\n") | |
| logger.debug(f"SSE raw line: {line!r}") | |
| if line == "": | |
| if any( | |
| event_line.startswith("event:") or event_line.startswith("data:") | |
| for event_line in event_lines | |
| ): | |
| async def _do_refresh(ref=self_ref): | |
| await asyncio.sleep(debounce_s) | |
| try: | |
| await ref.refresh() | |
| except Exception as e: | |
| logger.warning(f"SSE-triggered refresh failed: {e}") | |
| if debounce_task and not debounce_task.done(): | |
| debounce_task.cancel() | |
| debounce_task = asyncio.create_task(_do_refresh()) | |
| event_lines = [] | |
| continue | |
| if line.startswith(":"): | |
| continue | |
| event_lines.append(line) |
Copilot
AI
Apr 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When debouncing SSE events, the previous debounce_task is cancelled but never awaited/joined. Cancelled tasks can still surface "Task exception was never retrieved"/pending-task warnings depending on timing. Consider awaiting cancellation (with contextlib.suppress(asyncio.CancelledError)) or adding a done-callback that consumes exceptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
aiohttp StreamReader async for iteration yields lines or chunks?
💡 Result:
aiohttp StreamReader's default async for iteration (async for line in stream_reader:) yields lines (bytes ending with \n). For chunks, use explicit methods like iter_chunked(n), iter_any, or iter_chunks.
Citations:
- 1: https://aiohttp.readthedocs.org/en/stable/streams.html
- 2: https://aiohttp.readthedocs.io/en/stable/streams.html
- 3: https://docs.aiohttp.org/en/v3.7.4/streams.html
- 4: http://aiohttp.readthedocs.org/en/stable/_sources/streams.rst.txt
- 5: https://docs.aiohttp.org/en/v3.8.3/_sources/streams.rst.txt
🏁 Script executed:
# Find SuperpositionOptions class definition
find . -type f -name "*.py" | xargs grep -l "class SuperpositionOptions"Repository: juspay/superposition
Length of output: 121
🏁 Script executed:
# Search for _sse_connected_event usage in codebase
rg "_sse_connected_event" --type pyRepository: juspay/superposition
Length of output: 319
🏁 Script executed:
# Check SuperpositionOptions structure and org_id field
rg -A 20 "class SuperpositionOptions" --type pyRepository: juspay/superposition
Length of output: 1826
🏁 Script executed:
# Check if _sse_connected_event is awaited in initialize() method
rg "initialize" -A 20 --type py clients/python/provider/superposition_provider/local_provider.py | head -50Repository: juspay/superposition
Length of output: 1787
🏁 Script executed:
# Check types.py for org_id field definition and default values
cat clients/python/provider/superposition_provider/types.py | head -100Repository: juspay/superposition
Length of output: 2767
🏁 Script executed:
# View the full SSE loop code around line 540-570 to see blank line handling
sed -n '535,570p' clients/python/provider/superposition_provider/local_provider.pyRepository: juspay/superposition
Length of output: 2791
SSE loop: several small issues — unused _sse_connected_event, dead if org_id, inefficient per-line triggering, and broad except.
-
Line 516–517:
options.org_idis a required, non-defaultstrinSuperpositionOptions, soif options.org_id:is effectively dead code. Remove the conditional or confirm that empty strings are valid. -
Line 519–520:
self._sse_connected_eventis stored and.set()on connect but never awaited anywhere (noawait self._sse_connected_event.wait()ininitialize()or elsewhere). Either wire it intoinitialize()to block until the first SSE connection is established, or remove the field. -
Line 553–562 (SSE parsing): SSE events are delimited by blank lines, and a typical event has both
event:anddata:lines. The current code triggers_do_refreshon each matching line, creating two tasks per event—the first is immediately cancelled by the second, so it works due to the cancel-replace pattern, but it's inefficient and not correct SSE parsing. Buffer lines until the blank line terminator, then trigger once per event. -
Line 554–559:
except Exception as eis very broad (Ruff BLE001) but safe in this inner async context. Consider explicitly excludingCancelledErrorif needed. -
Line 535:
timeout=aiohttp.ClientTimeout(total=None, sock_read=30)means a 30s idle read will close the connection. Since the server emits keepalives every 15s, this is fine, but it couples client and server behavior tightly: if keepalive interval is raised past 30s, the connection will flap.
♻️ Suggested SSE parsing fix
- debounce_task = None
- async for line_bytes in resp.content:
+ debounce_task = None
+ pending_event = False
+ async for line_bytes in resp.content:
self_ref = weak_self()
if self_ref is None:
logger.info("Provider garbage collected, stopping SSE loop.")
return
line = line_bytes.decode("utf-8", errors="replace").strip()
logger.debug(f"SSE raw line: {line!r}")
- if not line or line.startswith(":"):
- continue
- if line.startswith("event:") or line.startswith("data:"):
+ if line.startswith(":"):
+ continue
+ if not line:
+ # event terminator — trigger once per event
+ if not pending_event:
+ continue
+ pending_event = False
async def _do_refresh(ref=self_ref):
await asyncio.sleep(debounce_s)
try:
await ref.refresh()
except Exception as e:
logger.warning(f"SSE-triggered refresh failed: {e}")
if debounce_task and not debounce_task.done():
debounce_task.cancel()
debounce_task = asyncio.create_task(_do_refresh())
+ elif line.startswith("event:") or line.startswith("data:"):
+ pending_event = True🧰 Tools
🪛 Ruff (0.15.10)
[warning] 558-558: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/python/provider/superposition_provider/local_provider.py` around
lines 508 - 569, The SSE loop has four fixes: remove or stop guarding header
addition by a dead check on options.org_id in _sse_loop (SuperpositionOptions
requires a non-empty org_id) or document that empty is allowed; either wire
self._sse_connected_event into initialize() to await the first connection (await
self._sse_connected_event.wait()) or remove the attribute and its set() call
(symbols: self._sse_connected_event, initialize); replace per-line handling in
_sse_loop/_do_refresh with proper SSE parsing by buffering resp.content lines
into an event until a blank line then trigger a single debounce refresh task per
complete event (symbols: _sse_loop, _do_refresh, debounce_task); and narrow the
broad inner except to avoid swallowing asyncio.CancelledError (catch
aiohttp.ClientError and asyncio.TimeoutError explicitly, and re-raise or let
CancelledError propagate). Optionally make sock_read timeout configurable rather
than hard-coding 30s if server keepalive cadence may change.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,6 +71,19 @@ class WatchStrategy: | |
| def default_watch_strategy(): | ||
| return WatchStrategy(500) | ||
|
|
||
| @dataclass | ||
| class SseStrategy: | ||
| """SSE-based refresh strategy. | ||
|
|
||
| Connects to the server's SSE endpoint and refreshes when a change event | ||
| is received. Reconnects automatically on connection failure. | ||
|
|
||
| Requires SuperpositionOptions to build the SSE endpoint URL and authenticate. | ||
| """ | ||
| superposition_options: SuperpositionOptions | ||
| reconnect_delay: int = 5 # seconds between reconnect attempts | ||
| debounce_ms: int = 500 # debounce rapid successive events | ||
|
|
||
|
Comment on lines
+74
to
+86
|
||
| @dataclass | ||
| class ManualStrategy: | ||
| """Manual refresh strategy. | ||
|
|
@@ -81,7 +94,7 @@ class ManualStrategy: | |
|
|
||
|
|
||
| # Union type for all refresh strategies | ||
| RefreshStrategy = Union[PollingStrategy, OnDemandStrategy, WatchStrategy, ManualStrategy] | ||
| RefreshStrategy = Union[PollingStrategy, OnDemandStrategy, WatchStrategy, SseStrategy, ManualStrategy] | ||
|
|
||
|
|
||
| # ============================================================================ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: use
get_running_loop()and rename ambiguousl.asyncio.get_event_loop()inside a coroutine is deprecated since Python 3.10 (and emits aDeprecationWarningin 3.12); in anasync def,asyncio.get_running_loop()is the idiomatic replacement. Also, the lambda parameterltrips Ruff E741 — rename toloop_arg(or_loop) for clarity.♻️ Suggested cleanup
🧰 Tools
🪛 Ruff (0.15.10)
[error] 30-30: Ambiguous variable name:
l(E741)
🤖 Prompt for AI Agents