feat(adms): add SAP ADMS module with sync/async clients and BDD tests#138
Open
adwitiyasushant wants to merge 30 commits into
Open
feat(adms): add SAP ADMS module with sync/async clients and BDD tests#138adwitiyasushant wants to merge 30 commits into
adwitiyasushant wants to merge 30 commits into
Conversation
|
|
Adds a full-featured ADMS (Attachment Document Management Service) module to the SDK with sync and async clients, IAS X.509 token authentication, OData V4 service support (DocumentService, ConfigurationService, AdminService), and pytest-bdd integration tests with Gherkin scenarios. Also adds shared SDK building blocks consumed by ADMS: IAS token fetcher, mTLS support, async HTTP client, and the ADMS telemetry module entry.
Required by upstream's "Enforce version bump when src/ is modified" CI check.
- Revert global [tool.pytest.ini_options] integration marker description and remove asyncio_mode=auto so the change does not leak into other modules' test runs. - Extract /etc/secrets/appfnd and CLOUD_SDK_CFG as module-level constants in adms/config.py for consistency with the existing _SERVICE_PATH / _ADMIN_SERVICE_PATH constants.
Upstream's data-anonymization PR (SAP#93) also bumped to 0.20.0 — bumping to 0.20.1 to satisfy the version-bump CI check on the rebased branch.
adwitiyasushant
force-pushed
the
contrib/adms
branch
from
2a3f60e to
74a9909
Compare
Required for pytest-asyncio strict mode (the project default after asyncio_mode auto was scoped out). Matches the convention already in use in tests/agentgateway/.
…trings Leftover wording from the dms module template — exception, config, auth and http docstrings now correctly identify themselves as ADMS. No behavioural change.
ArthurTonial
requested changes
May 27, 2026
- Add ADMS placeholder block to .env_integration_tests.example so reviewers know which env vars to set for external/BTP mode. - Correct the env var names in conftest docstring and INTEGRATION_TESTS_ADMS.md: the loader expects CLIENTID / CLIENTSECRET / URL / URI (matching the IAS binding field names used by destination/), not the underscored CLIENT_ID / IAS_URL / SERVICE_URL variants — those would have failed with KeyError. - Add the optional RESOURCE entry (IAS resource URI) the docs were missing. - Rename leftover "DMS Integration Tests" heading to "ADMS Integration Tests".
- Remove unused re-exports from adms/_auth.py (_CC_CACHE_KEY, _GRANT_JWT_BEARER) - Make InMemoryTokenCache thread-safe via threading.Lock - Drop module-specific reference from core/http async client docstring - Delete unused core/http/_batch.py and its tests/BDD scenarios - Remove 5 pattern YAML stubs from docs/adms/patterns/ - Restore underscore names for internal API classes in adms unit tests
Per PR review: integration tests target real service instances, so the ADMS-specific guide duplicates the canonical doc. Merge env vars into docs/INTEGRATION_TESTS.md and delete docs/INTEGRATION_TESTS_ADMS.md.
Per PR review. Local-only test driver — kept on developer machines via .gitignore but no longer shipped with the SDK.
… pattern Per PR review: - Drop the special CLOUD_SDK_ADMS_INTEGRATION_URL block from the workflow. ADMS now flows through the existing CLOUD_SDK_CFG_* loader like every other module. - Rewrite tests/adms/integration/conftest.py to target real ADM instances only. Removes the local HDM Maven auto-start mode (~150 lines), the mock IasTokenFetcher, and the CLOUD_SDK_ADMS_SKIP_IF_UNAVAILABLE flag. - Skip the suite gracefully when load_from_env_or_mount() raises ConfigError because of missing credentials. - Update docs and .env_integration_tests.example to match.
The concurrent-create integration test built coroutines and called asyncio.gather() from sync code, binding the resulting future to the default event loop. The run_async fixture then ran it in a separate loop, raising "future belongs to a different loop" against a real ADM instance. Wrap the gather in an async helper, mirroring the existing cleanup_concurrent_async_relations pattern, so coroutines and gather share the run_async fixture's loop.
…example Per PR review. The 'resource' field example pointed to a personal IAS application name; replace with the generic 'my-adm-app'.
Per PR review. Replace internal-only "Unified Provisioning / UCL" and "BTP Fabric SDK Business Services TRA" references with the generic "BTP service instance" wording suitable for a public SDK. Also fix the stale "DMS" tag on the first line; the module is ADMS.
Per PR review. The defensive untyped parameter and "avoid circular import" comment were incorrect — config.py imports only from core and exceptions, no cycle exists. Add the AdmsConfig import and annotate the parameter properly.
Per PR review. HttpError was imported twice — once bare and once aliased as AdmsHttpError. Since CoreHttpError is already aliased on the line below, the bare HttpError name is unambiguous. Drop the duplicate alias and use HttpError consistently.
Per PR review.
Per PR review. Replace `from sap_cloud_sdk.core.auth._token_cache import TokenCache` with the public re-export `from sap_cloud_sdk.core.auth import TokenCache` in client.py and __init__.py to avoid reaching into the private `_token_cache` module.
- Expand "ADM Document Management Service" to "Advanced Document Management Service" in _models.py docstring (consistent with the rest of the module). - Drop SAP-internal "Unified Provisioning / UCL" wording in user-guide.md; use the public "BTP service instance" phrasing already used in __init__.py.
Per PR review. * PEP 8: rename `mTLSConfig` → `MTLSConfig` and `mTLSStrategy` → `MTLSStrategy`. Public-facing classes now follow the standard PascalCase rule. All callers, BDD scenarios, step definitions, error messages and test class names are updated. * Drop the unused `client: Optional[httpx.AsyncClient] = None` parameter from `MTLSStrategy.apply_to_async_client`. The previous signature documented the argument as "Ignored (kept for API symmetry)" — it was never honoured because httpx does not allow swapping the SSL context on an existing client. The misleading parameter is removed and the docstring rewritten accordingly. Module docstring example and BDD step are updated to call without the argument.
Per PR review. `_CC_CACHE_KEY` and `_GRANT_JWT_BEARER` are private constants of `_ias_fetcher` and should not appear in the public `sap_cloud_sdk.core.auth` namespace. Drop them from the import list and `__all__` in core/auth/__init__.py, and update tests/adms/unit/test_auth.py to import `_CC_CACHE_KEY` directly from the private module (matching the existing pattern in tests/core/unit/auth/test_ias_fetcher.py).
# Conflicts: # pyproject.toml # uv.lock
* tests/adms/integration/test_e2e_async_flow.py — bind narrowed context.client / context.bo_type_id to local variables before the nested `_gather` async closure. ty re-widens optional attributes inside nested closures, so the outer-function asserts no longer apply once the closure references them. Local-variable binding preserves narrowing. * pyproject.toml + uv.lock — bump version 0.21.1 → 0.22.0. Required by check-version-bump CI now that the merge with main has introduced src/ changes (new ADMS module). Minor bump because this adds a new optional module.
betinacosta
reviewed
May 29, 2026
Adds one-line docstrings to 29 async methods in _AsyncDocumentApi, _AsyncDocumentRelationApi, and _AsyncConfigurationApi that referenced their sync siblings. Also corrects two module-header references from "DMS module" to "ADMS module" for consistency.
Replaces SAP-internal terminology (SPII, UCL callbacks) with neutral descriptions in _mtls.py since this module is part of the public SDK.
The Explicit Configuration example used outdated `base_url` / `token_url` parameters that no longer exist on AdmsConfig. Updated to the current `service_url` / `ias_url` fields.
Resolves three conflicts from the parallel agentgateway work landing on main: - .env_integration_tests.example — keep both ADMS and Agent Gateway env var blocks - docs/INTEGRATION_TESTS.md — keep both ADMS and Agent Gateway sections - tests/core/unit/telemetry/test_operation.py — combined operation count (4 agentgateway from upstream + 33 adms from this branch = 124)
Required by the CI version-bump check after merging upstream/main (which already shipped 0.22.0). Bumping minor since this PR still introduces the new ADMS module.
TokenCache is a generic auth utility from sap_cloud_sdk.core.auth and is not ADMS-specific. Re-exporting it from the adms namespace created two import paths for the same class. Users should import it directly from core.auth. Also drops the unused TokenCache import in user-guide.md (only RedisTokenCache was actually used in the example).
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
3 participants
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 new
sap_cloud_sdk.admsmodule providing a Python client for SAP'sAttachment Document Management Service (ADMS), along with the shared SDK
building blocks it depends on (IAS X.509 token fetcher, mTLS support, async
HTTP client, telemetry entry).
ADMS is the BTP service that lets agentic and business applications attach
documents to business object nodes (purchase orders, sales orders, etc.) with
virus scanning, scoped downloads, and audit metadata. This module is the
prerequisite for higher-level agent workflows that read/write documents on
behalf of users.
What's included
Public API (
sap_cloud_sdk.adms)AdmsClient(sync) andAsyncAdmsClientwith sub-namespaces:client.documents.*— document CRUD, content download URLs, scan-stategating
client.relations.*—DocumentRelationlifecycle (create / get / list /update / delete) plus draft create-validate-activate-discard flow
client.config.*—ConfigurationServicereads (allowed domains,document types, BO node types, type→BO mappings)
create_client()/create_async_client()factories that load the IASbinding from a mounted secret volume with env-var fallback.
Auth / HTTP
IasTokenFetcher(core.auth) — IASclient_credentialsflow with theresourceparameter that scopes the JWT'saudclaim to the ADMapplication.
AdmsHttp/ async variant — typedget/get_list/post/patch/deletehelpers with OData error → typed exception mapping(
DocumentNotFoundError,ScanNotCleanError,ConfigError, etc.).mTLShelpers incore.auth._mtls.Tests
mapping, and config resolution.
(
tests/adms/integration/document_flow.feature,async_flow.feature) — skipped when the service URL isn't configured.Module.ADMSto the telemetry enum.Why one squashed commit
This branch was rebased onto
mainand squashed before opening the PR toremove unrelated UCL prototype commits and version drift. Full review-by-review
history is preserved on my fork at
backup/contrib-adms-pre-rebase.Verification