Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion .claude/skills/review-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ compatibility: gh CLI ≥ 2.0, git, GitHub access to SAP/cloud-sdk-python

# PR Review: SAP Cloud SDK for Python

Reviews a PR against 23 criteria across 6 sections. Run from the root of the `cloud-sdk-python` repository.
Reviews a PR against 25 criteria across 6 sections. Run from the root of the `cloud-sdk-python` repository.

---

Expand Down Expand Up @@ -172,6 +172,16 @@ New list/query operations: encapsulate pagination params like existing modules (
**D5: Telemetry instrumentation**
New client methods: `@record_metrics(Module.X, Operation.Y)` from `core/telemetry`. New module: constant added to `core/telemetry/module.py` and operations to `core/telemetry/operation.py`. If module is called by other SDK modules: `_telemetry_source: Optional[Module] = None` param present.

**D6: Multi-tenancy support**
Comment thread
NicoleMGomes marked this conversation as resolved.
Multi-tenancy is a cross-cutting concern for most BTP applications. For new service modules, consider the full scope of multi-tenancy — not just token routing, but also provisioning and tenant isolation:

- **Provisioning:** Does the service require a subscription or onboarding step per tenant? Does the module need to support tenant lifecycle callbacks (subscribe/unsubscribe)?
- **Tenant isolation:** Is data or configuration isolated per tenant at the service level? Does the module enforce or expose tenant boundaries correctly?
- **Auth/routing:** Does the module need to route requests to a subscriber tenant context (e.g., XSUAA subdomain replacement, IAS `app_tid`, or Destination Service routing)?
- **Infrastructure:** Is there any infrastructure work required (e.g., new service binding fields, SPII fragments, Subscription Manager integration)?

This is a **nice-to-have** (⚠️ if absent, not ❌), but must be a conscious decision either way — the `user-guide.md` must document whether support is present, planned, or intentionally out of scope (see E2a).

---

### Section E: Tests & Documentation
Expand All @@ -182,6 +192,15 @@ Every changed `src/` file → corresponding change in `tests/`. Unit: `tests/[mo
**E2: Documentation quality**
New modules: `user-guide.md` with overview, quick start, config examples, API examples, troubleshooting. Changed public APIs: docstrings updated (Google/NumPy style: `Args:`, `Returns:`, `Raises:`). Sub-audience features not mixed into the general user guide.

**E2a: Multi-tenancy documentation**
Every new or modified `user-guide.md` must contain a `## Multi-tenancy` section with all four fields:
- **Supported:** Yes / No / Not yet / N/A
- **Authentication:** XSUAA / IAS / Other / —
- **How to use:** explanation (or "Not applicable")
- **Further reading:** link(s) or "—"

If the module exposes XSUAA or IAS authentication, the section must describe the actual mechanism (e.g., `access_strategy` param, `tenant` param, token URL replacement). A placeholder or missing section → ⚠️. If the module is a new service module with no section at all → ❌.

**E3: Module structure compliance**
New modules follow:
```
Expand Down Expand Up @@ -246,12 +265,14 @@ tests/[module]/integration/ (optional, BDD)
| D3 | Breaking changes marked | | |
| D4 | Pagination & tenant filtering | | |
| D5 | Telemetry instrumentation | | |
| D6 | Multi-tenancy support | | |

### E: Tests & Documentation
| # | Criterion | Status | Finding |
|---|-----------|--------|---------|
| E1 | Tests added/updated | | |
| E2 | Documentation quality | | |
| E2a | Multi-tenancy documentation | | |
| E3 | Module structure compliance | | |

---
Expand Down
7 changes: 7 additions & 0 deletions src/sap_cloud_sdk/adms/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,13 @@ activate_input = DraftActivateInput(
active = client.relations.activate_draft(activate_input)
```

## Multi-tenancy

- **Supported:** No
- **Authentication:** IAS
- **How to use:** Multi-tenancy is not supported by this service.
- **Further reading:** N/A

## Error Handling

```python
Expand Down
76 changes: 14 additions & 62 deletions src/sap_cloud_sdk/agent_memory/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ plain text, and the service makes it searchable by meaning.
- [`agent_id`](#agent_id)
- [`invoker_id`](#invoker_id)
- [Multitenancy](#multitenancy)
- [AccessStrategy](#accessstrategy)
- [Configuring at client level](#configuring-at-client-level)
- [SUBSCRIBER (default)](#subscriber-default)
- [PROVIDER](#provider)
- [Semantic Search: A Brief Primer](#semantic-search-a-brief-primer)
- [Memories](#memories)
- [Create a Memory](#create-a-memory)
Expand Down Expand Up @@ -172,64 +168,20 @@ across create, read, and search calls is the implementer's responsibility.

## Multitenancy

The Agent Memory service runs in a multi-tenant BTP environment. By default, every API
call uses a **subscriber-scoped token** — meaning data is isolated to the subscriber tenant
that your application serves. You control this behaviour with the `access_strategy` and
`tenant` keyword arguments available on every client method.
- **Supported:** Yes
- **Authentication:** XSUAA
- **How to use:** Pass `access_strategy` and `tenant` to `create_client()`. The strategy controls whether calls use a subscriber-scoped or provider-scoped XSUAA token. Every method on the client inherits the strategy set at construction time.

### AccessStrategy
| Value | Description |
| ------------------------------------- | ----------------------------------------------------------------------------------------------- |
| `AccessStrategy.SUBSCRIBER` (default) | Reads and writes against the subscriber tenant. Requires `tenant`. |
| `AccessStrategy.PROVIDER` | Reads and writes against the provider tenant. No `tenant` needed. Provides no tenant isolation. |

```python
from sap_cloud_sdk.agent_memory import AccessStrategy
```

| Value | Description |
| --------------------------- | ------------------------------------------------------------------------------------------------------------- |
| `SUBSCRIBER` (default) | Reads and writes against the subscriber tenant. Requires `tenant`. |
| `PROVIDER` | Reads and writes against the provider tenant. No `tenant` needed. Caution: this provides no tenant isolation. |

### Configuring at client level

Pass `access_strategy` and `tenant` to `create_client()` to set defaults for the entire
client instance. Every method call then inherits them, so you do not need to repeat them
on each operation.

```python
from sap_cloud_sdk.agent_memory import create_client, AccessStrategy

# Tenant set once — all calls below use it automatically
client = create_client(
access_strategy=AccessStrategy.SUBSCRIBER,
tenant="acme-corp",
)

memories = client.list_memories(agent_id="hr-assistant", invoker_id="user-42")
count = client.count_memories(agent_id="hr-assistant")
```
> [!WARNING]
> `PROVIDER` strategy provides **no tenant isolation**, the provider token grants access to data in the provider subaccount. Only use this strategy for provider-owned operations (e.g., admin tasks, shared datasets). Never use it to serve subscriber-specific data.
- **Further reading:** N/A

### SUBSCRIBER (default)

Configure a subscriber tenant at client creation. All calls will use that tenant context.

```python
client = create_client(
access_strategy=AccessStrategy.SUBSCRIBER,
tenant="acme-corp",
)
memories = client.list_memories(agent_id="hr-assistant", invoker_id="user-42")
```

### PROVIDER

Configure a provider-only client. No tenant is needed; all calls use the provider binding.

```python
client = create_client(access_strategy=AccessStrategy.PROVIDER)
memories = client.list_memories(agent_id="hr-assistant", invoker_id="user-42")
```

> [!WARNING]
> `PROVIDER` provides **no tenant isolation** — the provider token grants access to data across all subscriber tenants Only use this strategy for provider-owned operations (e.g., admin tasks, shared datasets). Never use it to serve subscriber-specific data.

## Semantic Search: A Brief Primer

Expand Down Expand Up @@ -568,10 +520,10 @@ See the [Content and metadata filtering](#content-and-metadata-filtering) note u

### Enums

| Enum | Values |
| ---------------- | -------------------------------------------- |
| `MessageRole` | `USER`, `ASSISTANT`, `SYSTEM`, `TOOL` |
| `AccessStrategy` | `SUBSCRIBER` (default), `PROVIDER` |
| Enum | Values |
| ---------------- | ------------------------------------- |
| `MessageRole` | `USER`, `ASSISTANT`, `SYSTEM`, `TOOL` |
| `AccessStrategy` | `SUBSCRIBER` (default), `PROVIDER` |

All models expose a `to_dict()` method that returns a plain dict for logging or forwarding.

Expand Down
23 changes: 16 additions & 7 deletions src/sap_cloud_sdk/agentgateway/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ mcp_tool_to_langchain(
The converter maps each property's JSON Schema `"type"` to the corresponding Python type so Pydantic validates and forwards the correct native type to the MCP server:

| JSON Schema type | Python type |
|------------------|-------------|
| ---------------- | ----------- |
| `"string"` | `str` |
| `"integer"` | `int` |
| `"number"` | `float` |
Expand All @@ -149,12 +149,21 @@ The SDK automatically detects the agent type based on the presence of a credenti

The SDK discovers resources via BTP Destination Service fragments filtered by the `sap-managed-runtime-type` label:

| Label value | Resource |
|---|---|
| `agw.mcp.server` | MCP tool server — `URL` property points to the MCP endpoint |
| `agw.a2a.server` | A2A agent — `URL` property is the agent base URL; ORD ID is extracted from the second-to-last URL path segment |
| `subscriber.ias` | IAS credential fragment for system-scoped token acquisition |
| `subscriber.ias.user` | IAS credential fragment for user-scoped token exchange |
| Label value | Resource |
| --------------------- | -------------------------------------------------------------------------------------------------------------- |
| `agw.mcp.server` | MCP tool server — `URL` property points to the MCP endpoint |
| `agw.a2a.server` | A2A agent — `URL` property is the agent base URL; ORD ID is extracted from the second-to-last URL path segment |
| `subscriber.ias` | IAS credential fragment for system-scoped token acquisition |
| `subscriber.ias.user` | IAS credential fragment for user-scoped token exchange |

## Multi-tenancy

- **Supported:** Yes (LoB flow); N/A (Customer flow)
- **Authentication:** IAS (IAS via Destination Service for LoB flow; mTLS for Customer flow)
- **How to use:**
Comment thread
NicoleMGomes marked this conversation as resolved.
- **LoB flow:** Pass `tenant_subdomain` to `create_client()`. All subsequent calls on that client instance use the subscriber tenant context.
- **Customer flow:** N/A
- **Further reading:** N/A

## API

Expand Down
8 changes: 8 additions & 0 deletions src/sap_cloud_sdk/aicore/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,14 @@ set_aicore_config()

---

## Multi-tenancy

- **Supported:** No
Comment thread
NicoleMGomes marked this conversation as resolved.
- **Authentication:** XSUAA (written to litellm environment variables)
- **How to use:** Not supported. This module is a process-level bootstrap that writes XSUAA credentials to environment variables consumed by litellm. Multi-tenant routing is out of scope at this layer.
- **Further reading:**
- [SAP AI Core — SAP Help Portal](https://help.sap.com/docs/sap-ai-core)

## Error Handling

Always handle potential configuration errors:
Expand Down
8 changes: 8 additions & 0 deletions src/sap_cloud_sdk/core/auditlog/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,14 @@ security_event = SecurityEvent(
client.log(security_event)
```

## Multi-tenancy

- **Supported:** N/A at auth level
- **Authentication:** XSUAA
- **How to use:** Tenant identity is expressed as an event-level field inside each logged event, not at the authentication layer. The provider always authenticates with its own XSUAA token. Pass `Tenant.SUBSCRIBER` or `Tenant.PROVIDER` when constructing events.
- **Further reading:**
- [SAP Audit Log Service — SAP Help Portal](https://help.sap.com/docs/btp/sap-business-technology-platform/audit-log-service)

## Error Handling

Always handle exceptions when logging audit events:
Expand Down
8 changes: 8 additions & 0 deletions src/sap_cloud_sdk/core/auditlog_ng/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,14 @@ with create_client(

---

## Multi-tenancy

- **Supported:** N/A at auth level
- **Authentication:** None (uses Destination Service / SPII for transport)
- **How to use:** Tenant identity is embedded in each event payload via the `tenant_id` field. Transport and auth are handled by the Destination Service / SPII. This module is only available through SAP for ME.
- **Further reading:**
- [SAP Audit Log Service — SAP Help Portal](https://help.sap.com/docs/btp/sap-business-technology-platform/audit-log-service)

## Validation

Events are validated against protobuf constraints using `protovalidate` before sending. A `ValueError` is raised if:
Expand Down
7 changes: 7 additions & 0 deletions src/sap_cloud_sdk/core/data_anonymization/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,13 @@ Typical file pseudonymization responses:
| `filename` | `str \| None` | Filename from the `Content-Disposition` header, if present. |
| `raw` | `dict` | Parsed JSON payload when available and suitable for response inspection. |

## Multi-tenancy

- **Supported:** No
- **Authentication:** N/A
- **How to use:** This module has no multi-tenancy model.
- **Further reading:** N/A

## Error Handling

Always catch `DataAnonymizationError` or its subclasses around calls:
Expand Down
7 changes: 7 additions & 0 deletions src/sap_cloud_sdk/core/secret_resolver/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,13 @@ max_retries = int(api_config.retries)

---

## Multi-tenancy

- **Supported:** N/A
- **Authentication:** N/A
- **How to use:** This is an infrastructure module for reading service bindings. It has no multi-tenancy concept of its own.
- **Further reading:** N/A

## Error Handling

The Secret Resolver handles missing secrets gracefully by leaving default values unchanged:
Expand Down
7 changes: 7 additions & 0 deletions src/sap_cloud_sdk/core/telemetry/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,13 @@ auto_instrument(middlewares=[StarletteIASTelemetryMiddleware(app=app)])

---

## Multi-tenancy

- **Supported:** N/A
- **Authentication:** N/A
- **How to use:** This is an infrastructure module. `set_tenant_id()` and `StarletteIASTelemetryMiddleware` allow attaching a tenant identifier to OpenTelemetry spans as metadata, but this is observability context.
- **Further reading:** N/A

## Configuration

### Production
Expand Down
8 changes: 8 additions & 0 deletions src/sap_cloud_sdk/destination/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,14 @@ mocks/certificates.json

Entries with a `"tenant"` field are treated as subscriber-specific. Entries without `"tenant"` are provider entries.

## Multi-tenancy

- **Supported:** Yes
- **Authentication:** XSUAA
- **How to use:** Pass `access_strategy` and `tenant` to the methods. The `AccessStrategy` enum controls whether the SDK fetches a subscriber-scoped or provider-scoped token by replacing the `identityzone` subdomain in the token URL.
- **Further reading:**
- [Multitenancy in the Destination Service — SAP Help Portal](https://help.sap.com/docs/CP_CONNECTIVITY/cca91383641e40ffbe03bdc78f00f681/4e07f250fe5d441cab09f69e22909198.html)

## Error Handling

- `DestinationNotFoundError`: mapped from HTTP 404 where applicable
Expand Down
14 changes: 5 additions & 9 deletions src/sap_cloud_sdk/dms/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -469,15 +469,11 @@ while True:

## Multi-Tenancy

All operations support an optional `tenant` parameter for subscriber-scoped requests. The SDK resolves the token URL by replacing the provider's identity zone with the tenant subdomain:

```python
# Provider context (default)
repos = client.get_all_repositories()

# Subscriber context
repos = client.get_all_repositories(tenant="subscriber-subdomain")
```
- **Supported:** Yes
- **Authentication:** XSUAA
- **How to use:** Pass an optional `tenant` parameter to any operation. The SDK resolves a subscriber-scoped XSUAA token by replacing the provider's `identityzone` subdomain in the token URL. Omitting `tenant` uses the provider token (default).
- **Further reading:**
- [Multitenancy with SAP Document Management Service — SAP Help Portal](https://help.sap.com/docs/DOCUMENT_MANAGEMENT/f6e70dd4bffa4b65965b43feed4c9429/0f6dd1bbaca342ee9177b9ece3fcaaa3.html)

---

Expand Down
8 changes: 8 additions & 0 deletions src/sap_cloud_sdk/objectstore/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,14 @@ client.delete_object("non-existent.txt") # This won't raise an error

---

## Multi-tenancy

- **Supported:** No (Object Store is not multi-tenant aware)
- **Authentication:** N/A
- **How to use:** Multi-tenancy is not supported by this service. Object Store uses static access key credentials. Each service binding is scoped to a single dedicated bucket. To serve multiple tenants, provision a separate service instance per tenant.
- **Further reading:**
- [SAP Object Store Service — SAP Help Portal](https://help.sap.com/docs/object-store)

## Error Handling

The ObjectStore module provides specific exceptions for different error scenarios:
Expand Down
8 changes: 8 additions & 0 deletions src/sap_cloud_sdk/print/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,14 @@ client.create_print_task(task)

---

## Multi-tenancy

- **Supported:** Not yet
- **Authentication:** XSUAA
- **How to use:** Multi-tenancy is not yet implemented in this module.
- **Further reading:**
- [SAP Print Service — SAP Help Portal](https://help.sap.com/docs/SCP_PRINT_SERVICE)

## Error Handling

The Print module provides specific exceptions for different error scenarios:
Expand Down
Loading