AWS, Core: Read S3 files via catalog-vended HTTPS URLs - #17457
Open
williamhyun wants to merge 4 commits into
Open
AWS, Core: Read S3 files via catalog-vended HTTPS URLs#17457williamhyun wants to merge 4 commits into
williamhyun wants to merge 4 commits into
Conversation
singhpk234
reviewed
Aug 1, 2026
singhpk234
left a comment
singhpk234
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the work @williamhyun !
Added first round of comments
Comment on lines
+75
to
+81
| if (httpClient == null) { | ||
| synchronized (this) { | ||
| if (httpClient == null) { | ||
| this.httpClient = HttpClients.custom().useSystemProperties().build(); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
follow-up : we would likely want to have these configurable rather than relying on SDK default or system props ?
Comment on lines
+59
to
+71
| private final StackTraceElement[] createStack; | ||
| private final CloseableHttpClient client; | ||
| private final String location; | ||
| private final String url; | ||
|
|
||
| /** Cached chunk buffer. {@code bufferFileStart} is the file offset of {@code buffer[0]}. */ | ||
| private byte[] buffer; | ||
|
|
||
| private long bufferFileStart = -1L; | ||
| private int bufferLimit = 0; | ||
|
|
||
| private long next = 0; | ||
| private boolean closed = false; |
Contributor
There was a problem hiding this comment.
lets integrate the metrics too, we need a counter to track bytes read ... like done in S3InputStream ?
check this : https://github.com/apache/iceberg/pull/17236/changes
Comment on lines
+41
to
+43
| return location != null | ||
| && (location.regionMatches(true, 0, "https://", 0, 8) | ||
| || location.regionMatches(true, 0, "http://", 0, 7)); |
Contributor
There was a problem hiding this comment.
i wonder if there is existing utils to extract scheme ? if yes can we use that ?
Member
Author
|
Thank you @singhpk234 for the comments, I have pushed with the most recent changes, please feel free to review again! |
williamhyun
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for reading object-store files over catalog-vended HTTP(S) URLs without requiring native cloud credentials on the reader, and wires it into
S3FileIO.HttpUrlSupport,HTTPInputFile, andHTTPInputStream— a seekable, range-readable HTTP(S) input path built on thehttpclient5dependencyiceberg-corealready depends on (also used byorg.apache.iceberg.rest.HTTPClient), so no new third-party dependency is added. Reads are served via chunked range GETs (4 MB chunks), with retries on transient network/server errors during those range fetches; content length is probed viaRange: bytes=0-0when unknown, since pre-signed GET URLs typically rejectHEAD.S3FileIO#newInputFile(String)/newInputFile(String, long)now short-circuit toHttpUrlSupportwhen the location is anhttp://orhttps://URL, leavings3:///s3a:///s3n://reads on the native, credentialed S3 client path unchanged.S3FileIO#close()now also closes the shared HTTP client.TestS3FileIOHttpUrlDispatch) and forHTTPInputFile/HTTPInputStreambehavior against a real localHttpServer(TestHTTPInputFile, including multi-chunk reads that cross the 4 MB boundary); a MinIO-backed integration test (TestS3FileIOPresignedUrls) that mints real pre-signed URLs and proves the native S3 client is never invoked; and an optional live-AWS integration test (TestS3FileIOLivePresignedUrls) gated on AWS credential env vars.This is the
FileIObuilding block for catalogs that vend a pre-signed URL as a file'sfile-pathin scan planning, so readers can fetch bytes over HTTPS with auth encoded in the URL itself. Delete-file handling, scan-planning integration, and other cloud providers are out of scope for this PR (see below).Scope / compatibility notes
file-path); the client uses that location unchanged as the fetch URL. There is no separate signing lookup or location-to-URL mapping in this PR.S3FileIOis updated.ResolvingFileIOand the GCS/ADLS equivalents are unchanged — a catalog that wants this behavior today needs to vendio-impl: S3FileIOper-table via the RESTLoadTableResponse.config()map (the specified mechanism for a catalog to select aFileIOfor a table), rather than relying onResolvingFileIO's scheme-based dispatch. Extending the other cloudFileIOs to the same short-circuit pattern is a natural follow-up.SplitScanTasksare already unsupported on the remote scan-plan read path, as are multiple deletion vectors in the same deletion file. These are known limitations of the remote scan-plan endpoint and is an orthogonal fix. (Core, REST: Fix delete file references for DVs in the same Puffin file #17497)Test plan
AWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY,AWS_REGION, andAWS_TEST_BUCKETset:./gradlew :iceberg-aws:integrationTest --tests org.apache.iceberg.aws.s3.TestS3FileIOLivePresignedUrlsAI Disclosure