Skip to content

Fix s3a multi-bucket S3 routing - #5240

Open
VectorPeak wants to merge 1 commit into
opendatalab:masterfrom
VectorPeak:codex-s3a-multi-bucket-routing
Open

Fix s3a multi-bucket S3 routing#5240
VectorPeak wants to merge 1 commit into
opendatalab:masterfrom
VectorPeak:codex-s3a-multi-bucket-routing

Conversation

@VectorPeak

@VectorPeak VectorPeak commented Jul 7, 2026

Copy link
Copy Markdown

Motivation

What Problem This Solves

MinerU's S3 path handling currently has two different ideas of what counts as an absolute S3 object-store URI.

parse_s3path() already treats both s3://bucket/key and s3a://bucket/key as valid S3-style paths. It strips query-like range parameters, splits the URI at ://, and returns the declared bucket and object key:

if is_s3_path(s3path):
    prefix, path = s3path.split('://', 1)
    bucket_name, key = path.split('/', 1)
    return bucket_name, key

Before this PR, the multi-bucket reader and writer only called that parser when the input started with s3://:

if path.startswith('s3://'):
    bucket_name, path = parse_s3path(path)
    s3_reader = self.__get_s3_client(bucket_name)
else:
    s3_reader = self.__get_s3_client(self.default_bucket)

That means a valid S3A path such as:

s3a://target-bucket/dir/file.pdf

was not handled as an absolute S3 URI. It fell into the relative-path branch, selected the configured default bucket, and could send the full s3a://target-bucket/dir/file.pdf string, optionally prefixed by default_prefix, as the object key.

For a multi-bucket reader configured with:

default bucket: default-bucket
default prefix: default-prefix
input path    : s3a://target-bucket/dir/file.pdf

The intended routing is:

bucket: target-bucket
key   : dir/file.pdf

The previous routing could become:

bucket: default-bucket
key   : default-prefix/s3a://target-bucket/dir/file.pdf

This is especially easy to hit when paths come from Hadoop, Spark, or data-lake workflows, where s3a:// is a common URI scheme for S3-compatible storage.

Changes

This PR adds a shared is_s3_path() helper backed by the same supported schemes used by parse_s3path():

S3_URI_SCHEMES = ('s3://', 's3a://')

MultiBucketS3DataReader.read_at() and MultiBucketS3DataWriter.write() now use that helper before dispatching to parse_s3path(). As a result, s3:// and s3a:// paths go through the same bucket/key parser, while ordinary relative paths still use the configured default bucket and optional default prefix.

The reader docstring is also updated to mention s3a:// as a supported input form, and focused unit tests cover the reader and writer S3A routing behavior without requiring live S3 credentials.

Evidence

The bug is visible at the dispatch boundary:

parse_s3path(s3a://target-bucket/dir/file.pdf)
  -> bucket target-bucket, key dir/file.pdf

MultiBucketS3DataReader.read_at(s3a://target-bucket/dir/file.pdf) before this PR
  -> does not call parse_s3path()
  -> uses default bucket branch

After this change, the same s3a:// input satisfies is_s3_path() and is routed through parse_s3path(), so the declared bucket and key are preserved.

Validation run locally on Windows with Python 3.12.13:

uvx ruff check mineru/data/utils/path_utils.py mineru/data/data_reader_writer/multi_bucket_s3.py tests/unittest/test_multi_bucket_s3.py
# All checks passed!

.\.venv-pr-s3a\Scripts\python.exe -m pytest -q tests/unittest/test_multi_bucket_s3.py
# 2 passed

.\.venv-pr-s3a\Scripts\python.exe -m py_compile mineru/data/utils/path_utils.py mineru/data/data_reader_writer/multi_bucket_s3.py
# passed

git diff --check
# passed

Possible call chain / impact

The affected multi-bucket read path is:

MultiBucketS3DataReader.read(s3a://target-bucket/dir/file.pdf?bytes=5,7)
  -> parse_s3_range_params(...)
  -> remove_non_official_s3_args(...)
  -> MultiBucketS3DataReader.read_at(s3a://target-bucket/dir/file.pdf, 5, 7)
  -> dispatches by URI scheme
  -> parse_s3path(...)
  -> __get_s3_client(target-bucket)
  -> s3_reader.read_at(dir/file.pdf, 5, 7)

The affected write path is:

MultiBucketS3DataWriter.write(s3a://target-bucket/out/file.md, data)
  -> dispatches by URI scheme
  -> parse_s3path(...)
  -> __get_s3_client(target-bucket)
  -> s3_writer.write(out/file.md, data)

This PR only changes S3 URI dispatch for s3a:// paths. It does not change low-level S3 client behavior, credentials, endpoint selection, range parsing, relative-path behavior, default bucket/default prefix behavior, or existing s3:// routing.

Modification

  • Add a shared is_s3_path() helper using the same supported URI schemes as parse_s3path().
  • Use that helper in both MultiBucketS3DataReader.read_at() and MultiBucketS3DataWriter.write() so s3a:// paths are parsed into their declared bucket/key.
  • Update the multi-bucket reader docstring to mention s3a:// paths.
  • Add focused unit tests for reader and writer s3a:// routing.

BC-breaking (Optional)

No backward compatibility break is expected. Existing s3:// paths and relative paths keep the same behavior. The only behavior change is that strings beginning with s3a:// are now treated as S3 URIs, matching the existing parse_s3path() contract, instead of being treated as default-bucket relative object keys.

Use cases (Optional)

This helps users who pass S3A-style paths from Hadoop, Spark, or data lake workflows into MinerU's multi-bucket S3 reader/writer:

s3a://target-bucket/documents/input.pdf

After this change, that path is routed to target-bucket with key documents/input.pdf instead of being interpreted under the configured default bucket.

Checklist

Before PR:

  • Pre-commit or other linting tools are used to fix the potential lint issues.
  • Bug fixes are fully covered by unit tests, the case that causes the bug should be added in the unit tests.
  • The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  • The documentation has been modified accordingly, like docstring or example tutorials.

After PR:

  • N/A - this narrow S3 URI routing fix does not require downstream project changes.
  • CLA has been signed and all committers have signed the CLA in this PR.

@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Jul 7, 2026
@VectorPeak
VectorPeak force-pushed the codex-s3a-multi-bucket-routing branch from 4ef5894 to 36c1113 Compare July 7, 2026 02:35
Route s3a:// paths through the same bucket/key parser used for s3:// paths so multi-bucket readers and writers do not treat them as default-bucket relative keys.

Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
@VectorPeak
VectorPeak force-pushed the codex-s3a-multi-bucket-routing branch from 36c1113 to e78ebca Compare July 7, 2026 12:28
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant