Skip to content

Commit 973075e

Browse files
committed
test(filesystem): expand remote emulator integration coverage
- Add xdist-safe, controller-managed S3, Azure, and GCS emulators with pinned images. - Cover remote formats, compression, globs, failures, native destination writes, cross-provider round trips, and S3 incremental loading. - Refresh S3 listings between repeated loads and support Azure destination connection strings. - Avoid emulator startup for unrelated targeted tests while keeping the fast lane Docker-free. - Document Azure connection-string authentication and repeated S3 load behavior.
1 parent 747b6aa commit 973075e

14 files changed

Lines changed: 604 additions & 189 deletions

File tree

docs/changelog.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22

33
## in progress
44

5+
- Filesystem: Refreshed S3 directory listings between repeated loads so
6+
file-level incremental runs detect newly uploaded objects.
7+
- Connectors: Added Azure Storage destination support for connection-string
8+
authentication and custom endpoints.
9+
- Connectors: Reject mixed Azure Storage credential modes instead of silently
10+
ignoring extra fields in source and destination URIs.
511
- Filesystem: Fail ingestion when a concrete source path matches no file while
612
keeping unmatched glob selections valid. Thanks, @hampsterx.
713
- Filesystem: Added rsync source connector. Thanks, @oferchen.

docs/supported-sources/azure-storage.md

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ abfss://?account_name=<your_account_name>&account_key=<your_account_key>
3434
## URI parameters
3535

3636
:account_name:
37-
Your Azure storage account name (required).
37+
Your Azure storage account name (required for account-key, SAS-token, and
38+
service-principal authentication). Do not supply it with `connection_string`.
3839

3940
:account_key:
4041
Your storage account access key (account-key auth).
@@ -47,14 +48,23 @@ abfss://?account_name=<your_account_name>&account_key=<your_account_key>
4748

4849
:account_host:
4950
Custom storage endpoint host (optional, for sovereign clouds or Azurite).
51+
Do not supply it with `connection_string`, which carries its own endpoints.
52+
53+
:connection_string:
54+
A complete Azure Storage connection string, as an alternative to the
55+
account-key, SAS-token, or service-principal fields. URL-encode the whole
56+
value before placing it in the URI. Connection strings can include custom
57+
service endpoints for local emulators such as Azurite.
5058

5159
:layout:
5260
Layout template (optional, destination only).
5361

54-
Supply **one** authentication mode: an `account_key`, a `sas_token`, or the
55-
full service-principal triplet (`tenant_id` + `client_id` + `client_secret`).
56-
Supplying both an account key/SAS and service-principal fields is rejected as
62+
Supply **one** authentication mode: a `connection_string`, an `account_key`, a
63+
`sas_token`, or the full service-principal triplet (`tenant_id` + `client_id` +
64+
`client_secret`). Supplying fields from multiple modes is rejected as
5765
ambiguous, and a partial service-principal triplet reports the missing field.
66+
When using `connection_string`, omit `account_name`, `account_host`, and all
67+
other credential fields.
5868

5969
:::{warning}
6070
Account keys are base64 (containing `+`, `/`, `=`) and SAS tokens embed their
@@ -63,6 +73,17 @@ own `&` and `=` characters. You must URL-encode credential values in the URI
6373
Unencoded values are mangled when the query string is parsed.
6474
:::
6575

76+
For example, write to an Azurite container by URL-encoding its connection
77+
string and passing it as the destination URI.
78+
79+
```sh
80+
omniload ingest \
81+
--source-uri 'csv:///local/users.csv' \
82+
--source-table 'users' \
83+
--dest-uri 'az://?connection_string=<url_encoded_connection_string>' \
84+
--dest-table 'my-container/users'
85+
```
86+
6687
:::{note}
6788
A Gen2 account differs from a plain Blob account only by having its
6889
hierarchical namespace enabled, so no separate configuration is required.
@@ -74,7 +95,7 @@ To integrate `omniload` with Azure Blob or Data Lake Storage, you need a
7495
storage account and one of the supported credentials. For guidance on
7596
obtaining an account key or SAS token, refer to the Microsoft documentation
7697
on [managing storage account access keys] and [shared access signatures].
77-
Service-principal credentials come from an [Azure AD app registration].
98+
Service-principal credentials come from an [Azure AD app registration].
7899

79100
## Authenticate with SAS token
80101

src/dlt_filesystem/source/impl/remote.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ def dlt_source(self, uri: str, table: str, **kwargs):
151151
fs_kwargs: dict = {
152152
"key": access_key_id[0],
153153
"secret": secret_access_key[0],
154+
# S3FileSystem caches directory listings by default. Disable the cache so
155+
# long-lived processes see objects created between incremental runs.
156+
"use_listings_cache": False,
154157
}
155158
if endpoint_url:
156159
fs_kwargs["endpoint_url"] = endpoint_url[0]

src/dlt_filesystem/target/remote.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,16 @@ def dlt_dest(self, uri: str, **kwargs):
5959
layout = params.get("layout", [None])[0]
6060
if layout is not None:
6161
opts["layout"] = layout
62+
filesystem_kwargs = self.filesystem_kwargs(params)
63+
if filesystem_kwargs:
64+
opts["kwargs"] = filesystem_kwargs
6265

6366
return BlobFS(**opts) # type: ignore
6467

68+
def filesystem_kwargs(self, params: dict) -> dict:
69+
"""Return extra fsspec constructor arguments for the destination."""
70+
return {}
71+
6572
def validate_table(self, table: str):
6673
table = table.strip("/ ")
6774
if len(table.split("/")) < 2:
@@ -163,6 +170,13 @@ def credentials(self, params: dict) -> FileSystemCredentials:
163170
"""
164171
auth = parse_azure_blob_auth(params)
165172

173+
if auth.connection_string is not None:
174+
# dlt does not model Azure connection strings in its credential union.
175+
# The actual credential is forwarded through filesystem_kwargs; a
176+
# resolved empty object prevents dlt from probing for unrelated account
177+
# fields before it constructs the adlfs filesystem.
178+
return AzureCredentials().resolve()
179+
166180
if auth.is_service_principal:
167181
# parse_azure_blob_auth guarantees the full triplet here; the
168182
# per-field `is not None` guards mirror the account-key branch and
@@ -191,6 +205,15 @@ def credentials(self, params: dict) -> FileSystemCredentials:
191205
key_credentials.azure_account_host = auth.account_host
192206
return key_credentials
193207

208+
def filesystem_kwargs(self, params: dict) -> dict:
209+
"""Forward connection-string credentials that dlt does not model."""
210+
auth = parse_azure_blob_auth(params)
211+
if auth.connection_string is None:
212+
return {}
213+
# adlfs does not forward api_version when constructing a client from a
214+
# connection string, so do not pass that parsed option here.
215+
return {"connection_string": auth.connection_string}
216+
194217

195218
class BlobFSClient(dlt.destinations.impl.filesystem.filesystem.FilesystemClient):
196219
@property

src/dlt_filesystem/util/auth.py

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ def parse_azure_blob_auth(params: dict) -> AzureBlobAuth:
5151
which ``parse_qs`` would otherwise mangle (``+`` becomes a space, an
5252
unencoded SAS token shatters into junk params).
5353
54-
Two auth modes are supported:
54+
Three auth modes are supported:
5555
56+
* connection string: ``connection_string``
5657
* account-key / SAS: ``account_key`` or ``sas_token``
5758
* service principal: the full ``tenant_id`` + ``client_id`` +
5859
``client_secret`` triplet
@@ -61,29 +62,43 @@ def parse_azure_blob_auth(params: dict) -> AzureBlobAuth:
6162
MissingValueError: if ``account_name`` is absent, if no auth material is
6263
supplied, or if the service-principal triplet is only partially
6364
supplied (naming the missing field(s)).
64-
ValueError: if mutually exclusive credentials are supplied together
65-
(``account_key`` with ``sas_token``, or account-key/SAS material
66-
with service-principal material), rather than silently picking one.
65+
ValueError: if mutually exclusive credentials are supplied together,
66+
rather than silently picking one.
6767
"""
6868

6969
def one(key: str) -> Optional[str]:
7070
return params.get(key, [None])[0]
7171

7272
connection_string = one("connection_string")
7373
api_version = one("api_version")
74+
account_name = one("account_name")
75+
account_key = one("account_key")
76+
sas_token = one("sas_token")
77+
sp_values = {field: one(field) for field in AZURE_SERVICE_PRINCIPAL_FIELDS}
78+
account_host = one("account_host")
7479

7580
if connection_string is not None:
81+
conflicting_fields = [
82+
field
83+
for field, value in {
84+
"account_name": account_name,
85+
"account_key": account_key,
86+
"sas_token": sas_token,
87+
"account_host": account_host,
88+
**sp_values,
89+
}.items()
90+
if value is not None
91+
]
92+
if conflicting_fields:
93+
raise ValueError(
94+
"Conflicting Azure credentials: connection_string cannot be "
95+
f"combined with {', '.join(conflicting_fields)}."
96+
)
7697
return AzureBlobAuth(
7798
connection_string=connection_string,
7899
api_version=api_version,
79100
)
80101

81-
account_name = one("account_name")
82-
account_key = one("account_key")
83-
sas_token = one("sas_token")
84-
sp_values = {field: one(field) for field in AZURE_SERVICE_PRINCIPAL_FIELDS}
85-
account_host = one("account_host")
86-
87102
if account_name is None:
88103
raise MissingConnectorOption("account_name", "Azure")
89104

tests/conftest.py

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,69 @@
1212

1313
from omniload.target.clickhouse import ClickhouseDestination
1414
from tests.util.common import get_testdata_path
15+
from tests.warehouse.manager import REMOTE_FILESYSTEM_SERVICES
1516
from tests.warehouse.settings import DESTINATIONS, SOURCES
1617

1718
logger = logging.getLogger(__name__)
1819

1920

21+
def _explicit_test_targets(config) -> list[Path]:
22+
"""Return explicit repo test paths from pytest's original invocation."""
23+
targets = []
24+
for arg in config.invocation_params.args:
25+
raw_path = arg.split("::", 1)[0]
26+
if raw_path.startswith("-"):
27+
continue
28+
path = Path(raw_path)
29+
if not path.is_absolute():
30+
normalized = raw_path.removeprefix("./")
31+
if normalized.split("/", 1)[0] not in {"tests", "examples", "omniload"}:
32+
continue
33+
path = Path(config.rootpath) / normalized
34+
targets.append(path.resolve())
35+
return targets
36+
37+
38+
def _only_main_filesystem_tests(config) -> bool:
39+
"""Return whether every explicit pytest target is in main/filesystem."""
40+
filesystem_root = (Path(config.rootpath) / "tests/main/filesystem").resolve()
41+
targets = _explicit_test_targets(config)
42+
return bool(targets) and all(
43+
target == filesystem_root or filesystem_root in target.parents
44+
for target in targets
45+
)
46+
47+
48+
def _remote_filesystem_tests_selected(config) -> bool:
49+
"""Return whether the invocation can collect the remote emulator test module."""
50+
remote_tests = (
51+
Path(config.rootpath) / "tests/main/filesystem/test_remote_integration.py"
52+
).resolve()
53+
targets = _explicit_test_targets(config)
54+
if not targets:
55+
return True
56+
return any(
57+
target == remote_tests or target in remote_tests.parents for target in targets
58+
)
59+
60+
61+
def _all_managed_containers():
62+
return (
63+
set(SOURCES.values())
64+
| set(DESTINATIONS.values())
65+
| set(REMOTE_FILESYSTEM_SERVICES.values())
66+
)
67+
68+
69+
def _managed_containers(config):
70+
containers = set()
71+
if not _only_main_filesystem_tests(config):
72+
containers |= set(SOURCES.values()) | set(DESTINATIONS.values())
73+
if _remote_filesystem_tests_selected(config):
74+
containers |= set(REMOTE_FILESYSTEM_SERVICES.values())
75+
return containers
76+
77+
2078
def pytest_configure(config):
2179
logging.getLogger("urllib3.connectionpool").setLevel(logging.INFO)
2280
logging.getLogger("testcontainers.core.waiting_utils").setLevel(logging.WARNING)
@@ -100,8 +158,10 @@ def testdata_path() -> Path:
100158

101159
@pytest.fixture(scope="session", autouse=True)
102160
def manage_containers(request, shared_directory):
103-
unique_containers = set(SOURCES.values()) | set(DESTINATIONS.values())
104-
for container in unique_containers:
161+
# Set every lock directory on both controller and workers. Container boot is
162+
# still filtered below, but fixture polling must not depend on both processes
163+
# deriving exactly the same target set from their invocation arguments.
164+
for container in _all_managed_containers():
105165
container.lock_dir = shared_directory # ty: ignore[invalid-assignment, unresolved-attribute, unused-ignore-comment]
106166

107167

@@ -111,8 +171,7 @@ def start_containers(config):
111171
if is_worker(config):
112172
return
113173

114-
unique_containers = set(SOURCES.values()) | set(DESTINATIONS.values())
115-
unique_containers = [x for x in unique_containers if x is not None]
174+
unique_containers = [x for x in _managed_containers(config) if x is not None]
116175

117176
for container in unique_containers:
118177
container.lock_dir = config.shared_directory # ty: ignore[invalid-assignment, unresolved-attribute, unused-ignore-comment]
@@ -164,8 +223,7 @@ def stop_containers(config):
164223
if not should_manage_containers:
165224
return
166225

167-
unique_containers = set(SOURCES.values()) | set(DESTINATIONS.values())
168-
unique_containers = [x for x in unique_containers if x is not None]
226+
unique_containers = [x for x in _managed_containers(config) if x is not None]
169227

170228
for container in unique_containers:
171229
try:

tests/dlt_filesystem/gcs.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ class GCSFakeServerContainer(DockerContainer):
3131
3232
>>> import google.cloud.storage
3333
>>> from google.auth.credentials import AnonymousCredentials
34-
>>> from testcontainers.gcs import GCSFakeServerContainer
34+
>>> from tests.dlt_filesystem.gcs import GCSFakeServerContainer
3535
36-
>>> with GCSFakeServerContainer() as container:
36+
>>> image = "docker.io/fsouza/fake-gcs-server:1.55.1"
37+
>>> with GCSFakeServerContainer(image) as container:
3738
... endpoint_url = container.get_endpoint_url()
3839
... client = google.cloud.storage.Client(
3940
... credentials=AnonymousCredentials(),
@@ -48,7 +49,7 @@ class GCSFakeServerContainer(DockerContainer):
4849

4950
def __init__(
5051
self,
51-
image: str = "docker.io/fsouza/fake-gcs-server:latest",
52+
image: str,
5253
*,
5354
service_port: int = 4443,
5455
**kwargs,

tests/dlt_filesystem/test_util.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import pytest
22

33
from dlt_filesystem.source.impl.util import strip_protocol_suffix
4+
from dlt_filesystem.util.auth import parse_azure_blob_auth
45
from dlt_filesystem.util.python import (
56
apply_alias,
67
asbool,
@@ -80,6 +81,29 @@ def test_apply_alias_collision():
8081
assert exc_info.match("use only one")
8182

8283

84+
@pytest.mark.parametrize(
85+
"field",
86+
[
87+
"account_name",
88+
"account_key",
89+
"sas_token",
90+
"account_host",
91+
"tenant_id",
92+
"client_id",
93+
"client_secret",
94+
],
95+
)
96+
def test_azure_connection_string_rejects_other_auth_fields(field):
97+
"""A connection string cannot silently override another Azure auth mode."""
98+
params = {
99+
"connection_string": ["UseDevelopmentStorage=true"],
100+
field: ["conflict"],
101+
}
102+
103+
with pytest.raises(ValueError, match=rf"cannot be combined with {field}"):
104+
parse_azure_blob_auth(params)
105+
106+
83107
def test_strip_protocol_suffix_success():
84108
"""Validate the `strip_protocol_suffix` utility function."""
85109
assert (

0 commit comments

Comments
 (0)