Skip to content

Commit f7ac9d0

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 7e3d5f8 commit f7ac9d0

12 files changed

Lines changed: 590 additions & 161 deletions

File tree

docs/changelog.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@
55
- Filesystem: Treat XLSX and ODS workbooks as multi-table sources by default,
66
while preserving one-table loads through explicit worksheet selectors.
77
Thanks, @hampsterx.
8+
- Filesystem: Refreshed S3 directory listings between repeated loads so
9+
file-level incremental runs detect newly uploaded objects.
10+
- Connectors: Added Azure Storage destination support for connection-string
11+
authentication and custom endpoints.
12+
- Connectors: Reject mixed Azure Storage credential modes instead of silently
13+
ignoring extra fields in source and destination URIs.
814
- Filesystem: Fail ingestion when a concrete source path matches no file while
915
keeping unmatched glob selections valid. Thanks, @hampsterx.
1016
- Filesystem: Added rsync source connector. Thanks, @oferchen.

docs/supported-sources/azure-storage.md

Lines changed: 31 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,28 @@ 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.
58+
59+
:api_version:
60+
Azure Storage API version override (optional, source only). Destination URIs
61+
ignore this parameter because adlfs does not forward it when constructing a
62+
client from a connection string.
5063

5164
:layout:
5265
Layout template (optional, destination only).
5366

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
67+
Supply **one** authentication mode: a `connection_string`, an `account_key`, a
68+
`sas_token`, or the full service-principal triplet (`tenant_id` + `client_id` +
69+
`client_secret`). Supplying fields from multiple modes is rejected as
5770
ambiguous, and a partial service-principal triplet reports the missing field.
71+
When using `connection_string`, omit `account_name`, `account_host`, and all
72+
other credential fields.
5873

5974
:::{warning}
6075
Account keys are base64 (containing `+`, `/`, `=`) and SAS tokens embed their
@@ -63,6 +78,17 @@ own `&` and `=` characters. You must URL-encode credential values in the URI
6378
Unencoded values are mangled when the query string is parsed.
6479
:::
6580

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

79105
## Authenticate with SAS token
80106

src/dlt_filesystem/source/impl/remote.py

Lines changed: 3 additions & 1 deletion
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]
@@ -293,7 +296,6 @@ class SFTPSource(FilesystemSource):
293296
"""Access files on SFTP servers."""
294297

295298
def dlt_source(self, uri: str, table: str, **kwargs):
296-
297299
if kwargs.get("incremental_key"):
298300
raise ValueError(
299301
"SFTP takes care of incrementality on its own, you should not provide incremental_key"

src/dlt_filesystem/target/remote.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,16 @@ def dlt_dest(self, uri: str, **kwargs):
6363
layout = params.get("layout", [None])[0]
6464
if layout is not None:
6565
opts["layout"] = layout
66+
filesystem_kwargs = self.filesystem_kwargs(params)
67+
if filesystem_kwargs:
68+
opts["kwargs"] = filesystem_kwargs
6669

6770
return BlobFS(**opts) # type: ignore
6871

72+
def filesystem_kwargs(self, params: dict) -> dict:
73+
"""Return extra fsspec constructor arguments for the destination."""
74+
return {}
75+
6976
def validate_table(self, table: str):
7077
table = table.strip("/ ")
7178
if len(table.split("/")) < 2:
@@ -167,6 +174,13 @@ def credentials(self, params: dict) -> FileSystemCredentials:
167174
"""
168175
auth = parse_azure_blob_auth(params)
169176

177+
if auth.connection_string is not None:
178+
# dlt does not model Azure connection strings in its credential union.
179+
# The actual credential is forwarded through filesystem_kwargs; a
180+
# resolved empty object prevents dlt from probing for unrelated account
181+
# fields before it constructs the adlfs filesystem.
182+
return AzureCredentials().resolve()
183+
170184
if auth.is_service_principal:
171185
# parse_azure_blob_auth guarantees the full triplet here; the
172186
# per-field `is not None` guards mirror the account-key branch and
@@ -195,6 +209,15 @@ def credentials(self, params: dict) -> FileSystemCredentials:
195209
key_credentials.azure_account_host = auth.account_host
196210
return key_credentials
197211

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

199222
class BlobFSClient(dlt.destinations.impl.filesystem.filesystem.FilesystemClient):
200223
@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: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,70 @@
1212

1313
from omniload.target.clickhouse import ClickhouseDestination
1414
from tests.util.common import get_testdata_path
15+
from tests.warehouse.manager import get_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 the collection paths resolved by pytest's argument parser."""
23+
targets = []
24+
for arg in config.args:
25+
raw_path = arg.split("::", 1)[0]
26+
path = Path(raw_path)
27+
if not path.is_absolute():
28+
normalized = raw_path.removeprefix("./")
29+
if normalized.split("/", 1)[0] not in {"tests", "examples", "omniload"}:
30+
continue
31+
path = Path(config.rootpath) / normalized
32+
targets.append(path.resolve())
33+
return targets
34+
35+
36+
def _only_main_filesystem_tests(config) -> bool:
37+
"""Return whether every explicit pytest target is in main/filesystem."""
38+
filesystem_root = (Path(config.rootpath) / "tests/main/filesystem").resolve()
39+
targets = _explicit_test_targets(config)
40+
return bool(targets) and all(
41+
target == filesystem_root or filesystem_root in target.parents
42+
for target in targets
43+
)
44+
45+
46+
def _remote_filesystem_tests_selected(config) -> bool:
47+
"""Return whether the invocation can collect the remote emulator test module."""
48+
if hasattr(config, "workerinput"):
49+
return config.workerinput["remote_filesystem_tests_selected"]
50+
if _integration_deselected(config):
51+
return False
52+
remote_tests = (
53+
Path(config.rootpath) / "tests/main/filesystem/test_remote_integration.py"
54+
).resolve()
55+
targets = _explicit_test_targets(config)
56+
if not targets:
57+
return True
58+
return any(
59+
target == remote_tests or target in remote_tests.parents for target in targets
60+
)
61+
62+
63+
def _all_managed_containers(config):
64+
containers = set(SOURCES.values()) | set(DESTINATIONS.values())
65+
if _remote_filesystem_tests_selected(config):
66+
containers |= set(get_remote_filesystem_services().values())
67+
return containers
68+
69+
70+
def _managed_containers(config):
71+
containers = set()
72+
if not _only_main_filesystem_tests(config):
73+
containers |= set(SOURCES.values()) | set(DESTINATIONS.values())
74+
if _remote_filesystem_tests_selected(config):
75+
containers |= set(get_remote_filesystem_services().values())
76+
return containers
77+
78+
2079
def pytest_configure(config):
2180
logging.getLogger("urllib3.connectionpool").setLevel(logging.INFO)
2281
logging.getLogger("testcontainers.core.waiting_utils").setLevel(logging.WARNING)
@@ -28,6 +87,9 @@ def pytest_configure(config):
2887
def pytest_configure_node(node):
2988
"""xdist hook"""
3089
node.workerinput["shared_directory"] = node.config.shared_directory
90+
node.workerinput["remote_filesystem_tests_selected"] = (
91+
_remote_filesystem_tests_selected(node.config)
92+
)
3193

3294

3395
def pytest_sessionstart(session):
@@ -68,6 +130,18 @@ def _integration_deselected(config):
68130
return (config.option.markexpr or "").strip() == "not integration"
69131

70132

133+
def pytest_ignore_collect(collection_path: Path, config) -> bool | None:
134+
"""Avoid importing emulator-only tests in the Docker-free fast lane."""
135+
if not _integration_deselected(config):
136+
return None
137+
remote_tests = (
138+
Path(config.rootpath) / "tests/main/filesystem/test_remote_integration.py"
139+
).resolve()
140+
if collection_path.resolve() == remote_tests:
141+
return True
142+
return None
143+
144+
71145
def _skip_containers(config):
72146
return _integration_deselected(config) or not _docker_available()
73147

@@ -100,8 +174,10 @@ def testdata_path() -> Path:
100174

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

107183

@@ -111,8 +187,7 @@ def start_containers(config):
111187
if is_worker(config):
112188
return
113189

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

117192
for container in unique_containers:
118193
container.lock_dir = config.shared_directory # ty: ignore[invalid-assignment, unresolved-attribute, unused-ignore-comment]
@@ -164,8 +239,7 @@ def stop_containers(config):
164239
if not should_manage_containers:
165240
return
166241

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

170244
for container in unique_containers:
171245
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,

0 commit comments

Comments
 (0)