Skip to content

feat(destination, agent-memory, core): support binding rotation in HTTP clients - #290

Draft
cassiofariasmachado wants to merge 7 commits into
mainfrom
refactor/support-binding-rotation-in-clients
Draft

feat(destination, agent-memory, core): support binding rotation in HTTP clients#290
cassiofariasmachado wants to merge 7 commits into
mainfrom
refactor/support-binding-rotation-in-clients

Conversation

@cassiofariasmachado

@cassiofariasmachado cassiofariasmachado commented Aug 27, 2026

Copy link
Copy Markdown
Member

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

Introduces a shared HttpClient + XsuaaAuthProvider layer in core/ that all service module transports compose. Solves a silent failure mode where long-lived clients hold frozen credentials — after a BTP secret rotation the old token expires and every call fails with 401 until the client is re-instantiated.

Two layers of rotation resilience:

  • Proactive: ConfigFactory.has_changed() checks the binding directory mtime before serving each cached token; on change it evicts the full token cache and re-reads credentials immediately.
  • Reactive: HttpClient.request invalidates the token cache entry and retries once on any 401 response.

Migrates both destination and agent_memory modules to compose HttpClient directly (removes HttpTransport from agent_memory; replaces TokenProvider in destination). Rewrites unit test suites for both modules to use module-level mock_http fixtures and a _make_response helper.

Related Issue

No linked issue.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Code refactoring
  • Dependency update

How to Test

  1. Run uv run pytest tests/agent_memory/ — 248 passed, 12 skipped
  2. Run uv run pytest tests/destination/ — all passing
  3. Run uvx pre-commit run --all-files — all hooks pass (ruff-format, ruff-check, ty)

Checklist

  • I have read the Contributing Guidelines
  • I have verified that my changes solve the issue
  • I have added/updated automated tests to cover my changes
  • All tests pass locally
  • I have verified that my code follows the Code Guidelines
  • I have updated documentation (if applicable)
  • I have added type hints for all public APIs
  • My code does not contain sensitive information (credentials, tokens, etc.)
  • I have followed Conventional Commits for commit messages

Breaking Changes

None. AgentMemoryClient and DestinationClient public APIs are unchanged. Internal HttpTransport class is removed but was never part of the public API.

Additional Notes

  • auditlog and data_anonymization modules are out of scope for this PR and can adopt HttpClient in a follow-up.
  • data_anonymization uses mTLS and would need an MTlsAuthProvider implementation.
  • ConfigFactory is generic (ConfigFactory[C]) and can be reused by any future module that needs binding re-read on rotation.

@cassiofariasmachado
cassiofariasmachado requested a review from a team as a code owner August 27, 2026 15:36
@cassiofariasmachado
cassiofariasmachado marked this pull request as draft August 27, 2026 15:43
@cassiofariasmachado
cassiofariasmachado force-pushed the refactor/support-binding-rotation-in-clients branch from c64beee to b85c7cb Compare August 27, 2026 16:29
Comment thread src/sap_cloud_sdk/agent_memory/_http.py Outdated
) from exc

return ConfigFactory(
module="hana-agent-memory",

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.

I believe we already have some enum for telemetry, can we extract it to core and reuse?

Comment thread src/sap_cloud_sdk/agent_memory/config.py Outdated
Comment thread src/sap_cloud_sdk/destination/__init__.py Outdated
@NicoleMGomes

NicoleMGomes commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Great solution!

Comment thread src/sap_cloud_sdk/destination/_http.py Outdated
Comment thread src/sap_cloud_sdk/core/_http_client.py
Comment thread src/sap_cloud_sdk/core/secret_resolver/_config_factory.py
Comment thread src/sap_cloud_sdk/core/_http_client.py Outdated
Comment thread src/sap_cloud_sdk/core/_http_client.py
@tiagoek

tiagoek commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

SDK Module Review

Check Status Findings
bdd ✅ PASS 0
binding-shape ✅ PASS 0
commits ✅ PASS 0
concurrency ✅ PASS 0
constants ⚠️ FLAG 3
deletion-hygiene ✅ PASS 0
deps-supply ✅ PASS 0
disclosure ✅ PASS 0
docs ✅ PASS 0
errors-logging ✅ PASS 0
hardcode ✅ PASS 0
http-hygiene ⚠️ FLAG 1
license-spdx ✅ PASS 0
patterns ⚠️ FLAG 1
pr-size ✅ PASS 0
quality-gate-parity ✅ PASS 0
secrets ✅ PASS 0
testing-depth ✅ PASS 0
versioning ✅ PASS 0

Findings (5)

5 finding(s): 5 posted as inline comment(s) on the affected lines, 0 not tied to a code line (listed above).


Generated by sdk-review-skill · v1

f"Request timed out: {method.value} {path}"
) from exc
except RequestException as exc:
raise AgentMemoryHttpError(

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.

[FLAG] PY-CON-01

String literal 'agentID' appears 3× — extract module-level constant

) from exc
except RequestException as exc:
raise AgentMemoryHttpError(
f"Request failed: {method.value} {path} — {exc}"

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.

[FLAG] PY-CON-01

String literal 'invokerID' appears 3× — extract module-level constant

except RequestException as exc:
raise AgentMemoryHttpError(
f"Request failed: {method.value} {path} — {exc}"
) from exc

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.

[FLAG] PY-CON-01

String literal 'content' appears 5× — extract module-level constant

base_url = getattr(self._auth_provider, "base_url", None) or self._base_url
else:
if self._plain_session is None:
self._plain_session = requests.Session()

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.

[FLAG] HTTP-01

HTTP session created per invocation — prefer single instance in init

_DEFAULT_INSTANCE = "default"


def _build_destination_http(

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.

[FLAG] PY-PT-08

Public function create_client missing type annotations: return type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants