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.
There was a problem hiding this comment.
I would expect something like create_client(tenant=<tenant>) where tenant is the subscriber agent sub-account id. Would it be possible? And of course you can also have something like:
create_client(config=..., tenant=...) where you can add manually the configuration and override SPII.
There was a problem hiding this comment.
create_client(tenant=subscriber_subaccount_id) would not work for two reasons:
- The CBC API today requires both
app_tenant_id(subscriber subaccount ID) andcbc_tenant_id. How the consumer derivescbc_tenant_idfromapp_tenant_idis outside this SDK's scope — it might be a DB lookup, a cache populated during SPII handshake, a call to UMS, etc. CBC may relax this in future (internally resolvecbc_tenant_idfrom the subaccount ID), but that's not supported today. - For multi-tenant agents,
tenantalso needs to be request-scoped — soCallable[[], TenantContext]is necessary alongside the direct value form.
There was a problem hiding this comment.
Since we agreed on moving tenant_context to client level, I've implemented this in 0c61a7f — TenantContext | Callable[[], TenantContext] at construction time, removed from the public method signatures. The reasoning for the callable and the two IDs is covered in the discussion above.
|
|
||
| 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?
There was a problem hiding this comment.
Yes please, remove local mode for now if it not a must have in V1.
There was a problem hiding this comment.
Done in c62dfa8 — _is_local_url deleted, user-guide, docstrings, and tests updated accordingly.
| 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?
There was a problem hiding this comment.
Yes, this sounds good. We just need to ensure that we are relying on what is already created on sap-internal-sdk during SPII.
fa9b989 to
c3dc18a
Compare
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
`_is_local_url` auto-disabled subdomain replacement for loopback URLs. Replace with an explicit opt-out: `replace_subdomain` now defaults to `True`; consumers set `CLOUD_SDK_CBC_REPLACE_SUBDOMAIN=false` when pointing at a local mock server. - Remove `_is_local_url` from `_http.py` - Default `replace_subdomain` to `True` in `DefaultClient.__init__` - Update `config.py` and `user-guide.md` to document the env-var escape hatch - Remove `TestDefaultClientLocalMode` and related tests
c3dc18a to
c62dfa8
Compare
Binds `TenantContext | Callable[[], TenantContext]` at construction time instead of per-call. The callable form supports multi-tenant agents where the tenant varies per request (e.g. read from a request-scoped context var). - `DefaultClient.__init__` and `create_client` gain `tenant_context` param - `get_consumption_versions` and `get_configuration` drop the param - `CBCClient` Protocol updated to match - Integration conftest bakes tenant into the client fixture - Tests cover callable invocation count and missing-tenant error
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 } ] } ] } ] }