feat(hindsight): add configurable HINDSIGHT_TIMEOUT env var#13067
Closed
tekgnosis-net wants to merge 2 commits into
Closed
feat(hindsight): add configurable HINDSIGHT_TIMEOUT env var#13067tekgnosis-net wants to merge 2 commits into
tekgnosis-net wants to merge 2 commits into
Conversation
The Hindsight Cloud API can take 30-40 seconds per request. The hardcoded 30s timeout was too aggressive and caused frequent timeout errors. This patch: 1. Adds HINDSIGHT_TIMEOUT environment variable (default: 120s) 2. Adds timeout to the config schema for setup wizard visibility 3. Uses the configurable timeout in both _run_sync() and client creation 4. Reads from config.json or env var, falling back to 120s default This makes the timeout upgrade-proof — users can set it via env var or config without patching source code. Signed-off-by: Kumar <kumar@tekgnosis.net>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a configurable timeout for the Hindsight memory provider to better accommodate slower Hindsight Cloud API operations, replacing the previously hardcoded client timeout behavior.
Changes:
- Introduces
_DEFAULT_TIMEOUT = 120and documentsHINDSIGHT_TIMEOUT. - Persists a
timeoutvalue in Hindsight provider config and exposes it viaget_config_schema(). - Applies the configured timeout when constructing the Hindsight cloud client.
Comments suppressed due to low confidence (1)
plugins/memory/hindsight/init.py:88
- The coroutine execution timeout is still fixed to
_DEFAULT_TIMEOUTbecause all call sites use_run_sync(...)without passing a timeout. This means users who setHINDSIGHT_TIMEOUThigher can still hitfuture.result(timeout=...)timeouts even if the HTTP client's request timeout is increased. Consider plumbing the configured provider timeout into these_run_synccalls (or making_run_syncderive its default from the provider’s resolved timeout) so both layers are consistent.
def _run_sync(coro, timeout: float = _DEFAULT_TIMEOUT):
"""Schedule *coro* on the shared loop and block until done."""
loop = _get_loop()
future = asyncio.run_coroutine_threadsafe(coro, loop)
return future.result(timeout=timeout)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…rations The previous commit added HINDSIGHT_TIMEOUT as a configurable env var, but _run_sync still used the hardcoded _DEFAULT_TIMEOUT (120s). All async operations (recall, retain, reflect, aclose) now go through an instance method that uses self._timeout, so the configured value is actually applied. Also: added backward-compatible alias comment for the module-level function.
Contributor
Author
|
The coroutine execution timeout issue is now fixed. What changed:
Files changed:
|
1 task
Collaborator
|
Likely duplicate of #10045 — both add configurable HINDSIGHT_TIMEOUT env var to the Hindsight memory provider. |
1 similar comment
Collaborator
|
Likely duplicate of #10045 — both add configurable HINDSIGHT_TIMEOUT env var to the Hindsight memory provider. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a configurable
HINDSIGHT_TIMEOUTenvironment variable to the Hindsight memory provider, replacing the previously hardcoded client timeout behavior.What Changed
Initial commit (6741b6c)
_DEFAULT_TIMEOUT = 120and documentsHINDSIGHT_TIMEOUTtimeoutvalue in Hindsight provider config and exposes it viaget_config_schema()Fix commit (0660c8f)
_run_sync()coroutine wrapper was still hardcoded to_DEFAULT_TIMEOUT(120s).HindsightMemoryProvider._run_sync(coro)instance method that delegates to the module-level function withtimeout=self._timeout_run_sync(...)toself._run_sync(...)HINDSIGHT_TIMEOUTvalueRelated Issues
How to Test
HINDSIGHT_TIMEOUT=180in~/.hermes/.envhindsight_reflectwith a query that takes >120s — it should now use the configured timeoutpytest tests/plugins/memory/test_hindsight_provider.py -v(all 46 tests pass)Platforms Tested
Notes
~/.hermes/hindsight/config.jsonand read on initialization