Conversation
|
|
||
| def get_configuration( | ||
| self, | ||
| tenant_context: TenantContext, |
There was a problem hiding this comment.
if tenant_context is required for all request, can we move it to client_level? Similar to agent gateway.
Also, can't we infer it from what is created during provisioning / spii?
There was a problem hiding this comment.
Thanks for the feedback. You're right that binding tenant context at client level is the better pattern — it's exactly what agentgateway does with tenant_subdomain: str | Callable[[], str]. The callable form is the key: one injected client instance, and the callable reads from whatever auth context the consumer maintains at call time (e.g. a request-scoped context var populated during SPII handling).
The TenantContext | Callable[[], TenantContext] signature handles both agent deployment modes:
- Single-tenant: tenant IDs are fixed at startup → pass a
TenantContextvalue directly. - Multi-tenant: tenant IDs vary per request → pass a callable that reads from the incoming request context.
One nuance worth aligning on: CBC requires two IDs per call — cbcTenantId (for URL subdomain routing) and appTenantId (query param). The app tenant is extractable from the auth context, but cbcTenantId comes from a separate mapping populated during SPII provisioning.
On inferring from SPII: the client can't own or infer that mapping — the SPII callback handler stores it wherever the consumer decides (a cache, a DB, a context var), and the client has no business coupling to that store. The consumer extracts both IDs and supplies them via the callable. The proposal would be:
client = create_client(
tenant_context=lambda: TenantContext(
cbc_tenant_id=get_cbc_tid(auth_ctx.tenant_id),
app_tenant_id=auth_ctx.tenant_id,
)
)
# then all call sites become:
config = client.get_configuration()Does that match your expectation? If so I'll update DefaultClient.__init__ to accept tenant_context: TenantContext | Callable[[], TenantContext] and remove it from the public method signatures — same pattern as agw's tenant_subdomain.
|
|
||
| Example (local mock):: | ||
|
|
||
| client = DefaultClient(base_url="http://localhost:8001") |
There was a problem hiding this comment.
We don't have a final decision about local mode and we would like to keep it consistent across module. Is this really needed on first version?
There was a problem hiding this comment.
You're right — on reflection this isn't needed. Quick context on why local mode exists: CBC rewrites the URL subdomain to the cbcTenantId on every request. When an agent tests against a locally running mock server (the CBC CLI's local cell command spins one up based on the agent's config object shapes), that rewrite silently corrupts the URL — http://localhost:8001 becomes http://<tenant-id>.localhost:8001, which won't resolve. Auto-detection was added to spare developers from having to know this.
But CLOUD_SDK_CBC_REPLACE_SUBDOMAIN=false alongside CLOUD_SDK_CBC_URL=http://localhost:8001 already handles it — so auto-detection is a convenience, not a necessity. Happy to remove it in v1 and revisit as part of the cross-module local mode decision. Shall I go ahead and remove it?
| TenantContext(cbcTenantId="t1", appTenantId="app-t1") | ||
| ) | ||
|
|
||
| Example (production):: |
There was a problem hiding this comment.
if we should have only create_client, why this is documented here?
We could also have a create_client receiving a config object for supporting more use cases in future. Check how other modules are doing it.
There was a problem hiding this comment.
Agreed — fixed in c3dc18a. Removed the contradictory direct-instantiation production example; create_client is now the only documented path. Note: if the client-level tenant_context suggestion is accepted, the docstring examples will be updated in the same change.
| replace_subdomain: bool | None = None | ||
|
|
||
|
|
||
| def load_from_env() -> CBCConfig: |
There was a problem hiding this comment.
Why this is only loading from env? This is not being provisioned by managed runtime. My expectation is that it should work similar to agw, where we read fragments and destination created during provisioning.
There was a problem hiding this comment.
Fair point — I see that aicore supports both:
- Destination mode:
AICORE_DESTINATION_NAMEset → fetches URL + credentials from BTP Destination Service at startup. - Direct mode (fallback): reads from mounted K8s secret volume or env vars — used for local development where no Destination Service is available.
CBC credentials are stored in a BTP Destination entry, so we should support the same pattern: CLOUD_SDK_CBC_DESTINATION_NAME → fetch URL + cert from the destination, with env vars as the local fallback. I'll double check how the CBC URL and credentials are stored in the destination and implement this mirroring the aicore approach — destination mode when CLOUD_SDK_CBC_DESTINATION_NAME is set, env/file fallback otherwise. Does that sound right?
Typed Python client for reading tenant-specific business configuration from SAP Central Business Configuration. Supports mTLS (production), local/mock (loopback auto-detection), and HTTPS mock servers via the CLOUD_SDK_CBC_REPLACE_SUBDOMAIN env var override. Public API: create_client(), CBCClient protocol, DefaultClient, CBCConfig, ConfigData / ConfigObject / EntityData / EntityContent, ConsumptionVersions, and a full CBC exception hierarchy.
…h params Replace the cert tuple parameter with symmetric cert_path/key_path params. Add CLOUD_SDK_CBC_CERT / CLOUD_SDK_CBC_KEY env vars so PEM values can be supplied directly (e.g. from K8s secrets) without writing to disk first — create_client() handles the temp-file lifecycle automatically.
…nts, version bump - Bump version to 0.54.0 (required by CI for src/ changes) - Fix ruff format violations in _models.py and client.py - Fix ty errors: conftest fixture return type CBCClient, test_models assert-not-None before .version - Update test_module (15→16) and test_operation (161→163) counts for CBC module/operations
- Soften "do not instantiate" to "prefer create_client" - Replace contradictory direct-instantiation examples with create_client usage - Reference BTP Destination Service and env vars as credential sources - Add tmp/ to .gitignore
fa9b989 to
c3dc18a
Compare
2 participants
Description
Adds
sap_cloud_sdk.cbc— a typed Python client for reading tenant-specific business configuration from SAP Central Business Configuration (CBC). Supports production (mTLS + subdomain-per-tenant URL routing), local mock servers (loopback auto-detection), with a full exception hierarchy, Pydantic-backed API models, and aCBCClientProtocol for test doubles.Related Issue
Closes #280
Type of Change
How to Test
Unit tests (no external service required):
Integration tests (requires a CBC server or mock):
Expected result: 51 unit tests pass; integration tests skip automatically when env vars are absent (CI-safe).
Checklist
Breaking Changes
None. This is a new module with no existing public API.
Additional Notes
Module structure follows the repo convention (
client.py,config.py,exceptions.py,_models.py,py.typed,user-guide.md).Key design decisions:
get_configurationgroups the flat entity list from the API intoConfigObjectbuckets, so consumers work with the authored config-object vocabulary rather than raw entity lists.EntityData,ConfigObject,ConfigDataare plain@dataclass(not Pydantic) — they are constructed in client code, never parsed from JSON.CLOUD_SDK_CBC_CERT_PATH/KEY_PATH) or PEM values (CLOUD_SDK_CBC_CERT/KEY) for environments where secrets are injected as env vars rather than mounted files.@record_metrics; internal helpers do not, to avoid double-counting a single user operation.Test evidence:
51 passed, 1 warning in 9.41s
Integration: 5 passed in 19.10s (real CBC server)
Sample ConfigData response (real CBC server)
{ "consumption_version": "a0392d4f-...", "tenant_context": { "cbc_tenant_id": "<cbc-tenant-id>", "app_tenant_id": "<app-tenant-id>" }, "config_objects": [ { "config_object_id": "payment-config", "entities": [ { "entity_id": "payment-mode", "data": [ { "paymentModeCode": "CASH", "name": "Cash", "isOnline": false }, { "paymentModeCode": "CARD", "name": "Credit / Debit Card", "isOnline": false }, { "paymentModeCode": "DIGITAL_WALLET","name": "Digital Wallet", "isOnline": true } ] } ] }, { "config_object_id": "tax-config", "entities": [ { "entity_id": "tax-category", "data": [ { "code": "STD", "ratePercent": 8.5, "isDefault": true }, { "code": "REDUCED", "ratePercent": 5, "isDefault": false }, { "code": "ZERO", "ratePercent": 0, "isDefault": false } ] } ] } ] }