Skip to content

feat!(objectstore): add Azure and GCS backends with provider auto-detection - #291

Draft
jplbrun wants to merge 9 commits into
mainfrom
feat/enable-gcs-and-azure-storages
Draft

feat!(objectstore): add Azure and GCS backends with provider auto-detection#291
jplbrun wants to merge 9 commits into
mainfrom
feat/enable-gcs-and-azure-storages

Conversation

@jplbrun

@jplbrun jplbrun commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

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

Adds Azure Blob Storage and Google Cloud Storage backends to the objectstore module and makes create_client() auto-detect the provider from the service binding at runtime. Previously the module only supported S3/MinIO.

The three backends are unified behind a single ObjectStoreClient protocol — a fixed contract of 8 methods (put_object_from_bytes, put_object, put_object_from_file, get_object, delete_object, list_objects, head_object, object_exists). Every backend ships all 8 from day one, so all providers are at parity and no caller has to branch on the provider. create_client() returns something satisfying this protocol; the concrete class you get back (S3Client, AzureClient, GcsClient) is an internal detail.

How a client is built:

  1. Auto-detection (default): create_client("object-store-1") reads the binding for that instance from the secret mount / env vars, inspects which keys are present, infers the provider, then loads and validates the full config.
  2. Explicit config (bypass detection): pass one of S3Config, AzureConfig, GcsConfig and the factory routes on its type — no binding read, no detection.

Key pieces added:

  • _protocol.py — the ObjectStoreClient protocol plus an ObjectReader protocol (the managed binary stream get_object returns).
  • _azure.py / _gcs.py — the two new backends.
  • _detect.py — reads whatever keys the binding presents and maps them to a provider (case-insensitive; providers are verified disjoint so a binding can't match two).
  • _factory.py — houses create_client (moved out of __init__.py; the public import path is unchanged because __init__.py re-exports it).
  • config.py — the three public *Config dataclasses plus internal *BindingData (what the secret resolver fills) with a validate() / to_config() step.
  • _validation.py — argument validation extracted out of _s3.py so all backends share it.

Related Issue

N/A

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

Unit tests

  1. uv run pytest tests/objectstore/unit -v — full unit suite for the module, including the new backends, detection, and config resolution:
    • test_detect.py — provider inference from binding keys + disjointness.
    • test_config.py — binding → config resolution and validation errors.
    • test_azure_client.py / test_gcs_client.py — the two new backends against mocked provider SDKs.
    • test_create_client.py — auto-detection, explicit-config routing, and error paths.
    • test_s3_client.py, test_models.py, test_protocol.py — updated for the rename/protocol.
  2. uv run pytest tests/ -m "not integration" — full unit suite. Expected: all pass.

Integration tests

  1. uv run pytest tests/objectstore/integration -m integration -v.

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

This PR reshapes the module's public surface. The impact on real consumers was measured against the agent-repo corpus; the affected population is small but non-zero, so each break is called out below with a migration.

1. ObjectStoreClient is now a Protocol, not an instantiable class

Before, ObjectStoreClient was the concrete S3 client and callers could construct it directly. It is now a typing.Protocol describing the shared interface, and the concrete S3 implementation moved to the private S3Client.

  • Breaks: any code that does ObjectStoreClient(...) directly (bypassing create_client).
  • Still works: using ObjectStoreClient purely as a type annotationcreate_client() still returns something that satisfies it, so x: ObjectStoreClient = create_client(...) type-checks unchanged.
  • Migration: build clients through create_client(instance) (auto-detection) or create_client(instance, config=S3Config(...)) (explicit). Do not instantiate the client class yourself.
# before
from sap_cloud_sdk.objectstore import ObjectStoreClient
client = ObjectStoreClient(creds_config)

# after
from sap_cloud_sdk.objectstore import create_client, S3Config
client = create_client("object-store-1", config=S3Config(...))

2. ObjectStoreBindingData is renamed and no longer public

The single S3-shaped ObjectStoreBindingData dataclass is gone from the public API. It is replaced by three provider-specific public config types — S3Config, AzureConfig, GcsConfig — which are what create_client(config=...) accepts and what __all__ exports. (Provider-specific *BindingData types still exist internally as secret-resolver targets, but they are not part of the public surface.)

  • Breaks: any code importing ObjectStoreBindingData, whether from the public re-export or reaching into sap_cloud_sdk.objectstore._models.
  • Migration: replace with the matching *Config. The S3 field names are unchanged (access_key_id, secret_access_key, bucket, host), with disable_ssl now living on S3Config instead of on create_client (see chore(deps): update setuptools requirement from ~=80.9.0 to >=80.9,<82.1 #4).
# before
from sap_cloud_sdk.objectstore import ObjectStoreBindingData
cfg = ObjectStoreBindingData(access_key_id=..., secret_access_key=..., bucket=..., host=...)

# after
from sap_cloud_sdk.objectstore import S3Config
cfg = S3Config(access_key_id=..., secret_access_key=..., bucket=..., host=...)

3. create_client(config=...) now takes a typed union, not ObjectStoreBindingData

The config keyword's type changed from Optional[ObjectStoreBindingData] to Union[S3Config, AzureConfig, GcsConfig, None]. The factory routes on the concrete config type to pick the backend. This is a direct consequence of #2 — a caller passing ObjectStoreBindingData no longer compiles/runs.

4. disable_ssl moved off create_client onto S3Config

disable_ssl was a keyword on create_client, but it only ever mapped to MinIO's plain-HTTP mode — it does nothing for Azure (URI fixes the scheme to https) or GCS (always HTTPS). It's now a field on S3Config.

  • Breaks: create_client(instance, disable_ssl=True) — the kwarg no longer exists, so this raises TypeError at runtime.
  • Migration: set it on the config. Note that disable_ssl=True requires the explicit-config path — there is no way to enable it while relying on auto-detection, because auto-detection builds the config from the binding and the binding resolver only handles str fields. You must supply full credentials via S3Config when you need plaintext mode (this is a local-dev / MinIO-only scenario in practice).
# before
client = create_client("object-store-1", disable_ssl=True)

# after — must supply full credentials explicitly
from sap_cloud_sdk.objectstore import create_client, S3Config
client = create_client(
    "object-store-1",
    config=S3Config(
        access_key_id="...",
        secret_access_key="...",
        bucket="...",
        host="localhost:9000",
        disable_ssl=True,
    ),
)

5. get_object() return type changed: http.client.HTTPResponseObjectReader

get_object previously returned MinIO's raw urllib3/HTTPResponse object; it now returns an ObjectReader — a small protocol exposing read(), close(), and context-manager support. This was necessary to give all three providers a common, provider-agnostic return type.

  • Breaks: any caller that annotated the result as HTTPResponse, or that relied on HTTPResponse-specific attributes/methods beyond read()/close() (e.g. .status, .getheaders(), .release_conn()).
  • Migration: treat the result as a plain readable binary stream, ideally via with:
with client.get_object("report.pdf") as r:
    data = r.read()

Additional Notes

N/A

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant