Add S3 storage integration tests with a mock S3 server

[jantonguirao]

Category:

Other (Tests, Documentation)

Description:

DALI's s3:// support had no test coverage. This adds integration tests that start a mock S3 server
in the background, seed a bucket, and assert that the readers return byte-identical results to the
same data read from local disk.

The mock server is moto, started and torn down by the test module itself, so no external service or
CI-side fixture is needed. DALI_TEST_S3_ENDPOINT points the same tests at MinIO or at real S3
instead.

Two implementation details are load-bearing and are commented in the source so they are not
"simplified" away later:

• The server runs in a subprocess, not in-process. Pipeline::Build is bound without
  py::call_guard<py::gil_scoped_release>, and S3 object listing happens inside Build(), so an
  in-process server thread is GIL-starved and the request eventually times out in libcurl
  (curlCode: 28). Every moto example shows the in-process form, hence the comment.
• The endpoint is always an IP literal. DALI does not set useVirtualAddressing, so the AWS SDK
  only selects path-style addressing when the endpoint host is an IP. With a host name the bucket is
  prepended to the host instead, which requires a matching DNS entry (and MINIO_DOMAIN, for a real
  MinIO).

Additional information:

Affected modules and functionalities:

• dali/test/python/s3_test_utils.py (new): mock server lifecycle, env setup, bucket helpers and
  skip predicates.
• dali/test/python/reader/test_s3.py (new): the tests.
• qa/TL0_python-self-test-readers-decoders/test_nofw.sh: adds moto, flask, flask-cors and
  boto3 to pip_packages. Plain moto does not pull in flask; that lives in the moto[server]
  extra, which also drags in cfn-lint, docker and more.
• qa/TL0_python-self-test-readers-decoders/test_body.sh: skips the module under sanitizers.
• docs/env_vars.rst: documents AWS_ENDPOINT_URL, which the tests rely on and which was
  previously undocumented.

Key points relevant for the review:

• No DALI source changes are needed - S3ClientManager already reads AWS_ENDPOINT_URL.
• The suite already runs nose2 over dali/test/python/reader, so the module is picked up by
  discovery and only the pip package list needed changing.
• The module skips cleanly when moto is absent, so the conda and tegra suites, which run the same
  directory without it, stay green with no edits.
• It is skipped under sanitizers because qa/leak.sup has no suppressions for the AWS SDK, libcurl
  or OpenSSL globals, and the mock server subprocess would inherit LD_PRELOAD.
• Signing is deliberately not covered. moto does not verify SigV4, so the key-with-a-space case
  pins URL path construction rather than canonical-request encoding. Signing fidelity needs a run
  against MinIO or real S3 through DALI_TEST_S3_ENDPOINT; worth a follow-up job.
• Cost is about 16 s, CPU only (device_id=None), on a suite that already runs for several minutes.
  Roughly 9 s of that is the listing-pagination case, which seeds 1100 objects.

Tests:

• Existing tests apply
• New tests added
  • Python tests
  • GTests
  • Benchmark
  • Other
• N/A

New module reader/test_s3.py, covering:

Test	What it exercises
test_file_reader_file_root	ListObjectsV2 with a prefix, label assignment, file_filters over keys
test_file_reader_files_arg	HeadObject + ranged GetObject, a key containing a space, a 0-byte object
test_file_reader_listing_pagination	continuation tokens across the 1000-key page limit (1100 keys)
test_webdataset_index_inferred	index inferred from the tar, i.e. many small ranged GETs
test_webdataset_local_index	S3 shard with a local index file
test_file_reader_missing_object	error surface for a missing key
test_file_reader_missing_bucket	error surface for a missing bucket

Readers that do not route through FileStream::Open (FITS, LMDB-backed caffe/caffe2, nemo_asr,
sequence, video) are out of scope. fn.readers.numpy over s3:// is also left out of this PR - it
hits a separate bug, fixed in DALI-4887; it can be added to this matrix once that lands.

Checklist

Documentation

• Existing documentation applies
• Documentation updated
  • Docstring
  • Doxygen
  • RST
  • Jupyter
  • Other
• N/A

DALI team only

Requirements

• Implements new requirements
• Affects existing requirements
• N/A

REQ IDs: N/A

JIRA TASK: DALI-4010

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[greptile-apps]

Confidence Score: 4/5

The PR appears safe to merge after addressing the required RST formatting rule; bounding mock-server startup and isolating external test data are worthwhile non-blocking reliability improvements.

Findings

1.  Unbounded startup wait ▶
2.  External data is persistent ▶
3.  Literal uses wrong markup ▶

Summary

• Adds mock and external S3 server helpers, credential setup, and bucket seeding.
• Covers listing pagination, ranged reads, spaces in keys, empty objects, WebDataset indices, and missing-resource errors.
• Installs the mock-server dependencies in the reader/decoder QA suite and skips the module under sanitizers.
• Documents AWS_ENDPOINT_URL and its process-wide lifetime.
• The principal follow-ups are bounding the initial subprocess handshake and isolating/cleaning external test data.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[setUpModule] --> B{DALI_TEST_S3_ENDPOINT set?}
  B -->|No| C[Start moto subprocess]
  B -->|Yes| D[Use external S3 or MinIO]
  C --> E[Export AWS environment]
  D --> E
  E --> F[Create bucket and upload fixtures]
  F --> G[Build DALI S3 reader pipelines]
  G --> H[Compare with local reader outputs]
  G --> I[Verify listing pagination and errors]
  H --> J[tearDownModule]
  I --> J
  J --> K[Stop mock server and clean local files]

Loading

_{Reviews (1) · Last reviewed commit: "Add S3 storage integration tests with a ..."}

[greptile-apps]

Unbounded startup wait

This blocking readline() runs before the startup deadline is created. If importing moto or starting ThreadedMotoServer stalls before the child prints its port, the test process waits indefinitely and never reaches _wait_until_serving(). Please read the port with a bounded mechanism and terminate the child when startup_timeout_s expires.

[greptile-apps]

External data is persistent

External MinIO or S3 runs upload data under fixed data/, wds/, and many/ prefixes, but teardown only cleans local resources. If the bucket is shared or was used by an earlier run, extra keys can break the exact 1,100-object assertion, concurrent runs can interfere, and each run leaves test objects behind. Please isolate each run with a unique prefix and remove its remote objects during teardown.

[greptile-apps]

Literal uses wrong markup

The new heading uses single backticks around AWS_ENDPOINT_URL. The repository's RST directive requires double backticks for literal or inline code, so this formatting requirement must be satisfied before merging.

Rule Used: RST formatting: single backticks for cross-reference roles (:ref:, :py:obj:) and named external links, double backticks for literal/inline code. Don't mix the two — using single backticks for what should be a code literal renders as italics, and ... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[Copilot]

🟡 Changes recommended

Unresolved critical unsupported-build handling and multiple moderate external-S3 lifecycle and configuration issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds S3 reader integration tests using a subprocess-based Moto server, with optional external S3-compatible endpoints.

Changes:

• Adds S3 lifecycle, seeding, environment, and capability helpers.
• Tests file readers, WebDataset readers, pagination, and error handling.
• Updates dependencies, sanitizer exclusions, and AWS_ENDPOINT_URL documentation.

File summaries

File	Changes and review comments
qa/TL0_python-self-test-readers-decoders/test_nofw.sh	Adds Moto, Flask, Flask-CORS, and boto3 dependencies.
qa/TL0_python-self-test-readers-decoders/test_body.sh	Skips the S3 module under sanitizers.
docs/env_vars.rst	Documents AWS_ENDPOINT_URL.
dali/test/python/s3_test_utils.py	Provides S3 server and setup helpers. Critical (2 votes): the unsupported-build probe does not exercise FileStream::Open, so BUILD_AWSSDK=OFF builds fail instead of skipping. Moderate: external runs do not reliably handle missing boto3 (1), overwrite standard AWS credentials (2), can hang before the startup timeout (1), force us-east-1 (1), and do not support external-region bucket creation (1).
dali/test/python/reader/test_s3.py	Adds file-reader and WebDataset tests, pagination, and missing-resource checks. Moderate: external runs leave seeded objects behind (2) and rely on a potentially pre-existing missing-bucket name (1).

Review details

Suppressed comments (4)

dali/test/python/reader/test_s3.py:191

• The negative test relies on dali-no-such-bucket never existing, but that is not guaranteed when DALI_TEST_S3_ENDPOINT points at a persistent MinIO or real S3 service (the name could already be present or owned by the test account). In that case listing succeeds or returns a different error and the NoSuchBucket assertion fails. Use a per-run unique bucket name or make the missing-bucket case configurable/skip it for external services.

def test_file_reader_missing_bucket():
    with assert_raises(RuntimeError, glob="*NoSuchBucket*"):
        file_pipe(file_root="s3://dali-no-such-bucket/data").build()

dali/test/python/s3_test_utils.py:193

• When DALI_TEST_S3_ENDPOINT is set, this returns before checking boto3, but setUpModule still calls s3_client() and that function unconditionally imports boto3 at line 166. Consequently an external MinIO/real-S3 run without boto3 fails with an uncaught import error instead of being skipped; check boto3 unconditionally and only make moto.server conditional on the absence of the external endpoint.

    if os.environ.get("DALI_TEST_S3_ENDPOINT"):
        return

dali/test/python/s3_test_utils.py:94

• This blocking readline() is executed before _wait_until_serving() applies startup_timeout_s. If moto import or server.start() hangs before printing a port, the whole reader suite hangs indefinitely rather than failing after the configured 60 seconds; read/poll the pipe with the deadline (or add a watchdog) before waiting for the startup line.

        line = self._proc.stdout.readline()  # the child reports the port it bound

dali/test/python/s3_test_utils.py:29

• REGION is forced to us-east-1 and then exported over both standard region variables, so the advertised external real-S3 mode cannot use a bucket in another region: the unconditional create_bucket call will require a location constraint, and DALI requests will be signed with the wrong region. Allow the external test region to be configured and create the bucket with the matching LocationConstraint when needed.

REGION = "us-east-1"

• Files reviewed: 5/5 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jantonguirao]

!build

[dali-automaton]

CI MESSAGE: [67423372]: BUILD STARTED

[dali-automaton]

CI MESSAGE: [67423372]: BUILD FAILED