GH-50126: [C++] Make S3Filesystem consume key-value options by raulcd · Pull Request #50128 · apache/arrow

raulcd · 2026-06-08T15:48:22Z

TBD once the PR is not on draft anymore

This is a follow up PR from #50044 currently only opened to exercise CI.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

This PR includes breaking changes to public APIs. (If there are any breaking changes to public APIs, please explain which changes are breaking. If not, you can remove this.)

This PR contains a "Critical Fix". (If the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld), please provide explanation. If not, you can remove this.)

GitHub Issue: [C++] Make S3Filesystem consume key-value options #50126

…terface

…umbing for ARROW_S3_MODULE to CI

github-actions · 2026-06-08T15:49:54Z

⚠️ GitHub issue #50126has been automatically assigned in GitHub to PR creator.

Copilot

Pull request overview

This PR extends the C++ filesystem factory plumbing so FileSystemFromUri can pass backend-specific key/value options through to registered factories, and updates the S3 filesystem factory to actually consume a subset of those options (notably credentials) when constructing S3Options.

Changes:

Thread backend-specific std::any options through FileSystemFactory / FileSystemFromUri and add new FileSystemFromUri overloads accepting options.
Add S3Options::FromUriAndOptions(...) and update the S3 filesystem factory registration to use it.
Update/extend tests and CI/CMake so module tests are built and exercised when appropriate.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
cpp/src/arrow/testing/examplefs.cc	Updates the example filesystem factory to accept options and validate/consume them.
cpp/src/arrow/filesystem/s3fs.h	Adds `S3Options::FromUriAndOptions` declarations and includes needed for `std::any`.
cpp/src/arrow/filesystem/s3fs.cc	Implements `FromUriAndOptions` and wires S3 factory registration to consume options.
cpp/src/arrow/filesystem/s3fs_module_test.cc	Adds module-level test coverage around options passed to S3 via `FileSystemFromUri`.
cpp/src/arrow/filesystem/localfs_test.cc	Extends tests to validate options forwarding to dynamically loaded factories; adjusts registrar construction for nullptr cases.
cpp/src/arrow/filesystem/filesystem.h	Adds options-aware factory signature and new `FileSystemFromUri` overload declarations + docs.
cpp/src/arrow/filesystem/filesystem.cc	Plumbs options through the `FileSystemFromUri*` implementations and rejects options for non-factory schemes.
cpp/src/arrow/filesystem/filesystem_test.cc	Adds a test asserting non-factory schemes reject non-empty options.
cpp/src/arrow/filesystem/CMakeLists.txt	Gates `s3fs_module_test` on `ARROW_BUILD_TESTS` in addition to `ARROW_S3_MODULE`.
ci/scripts/cpp_build.sh	Ensures `ARROW_S3_MODULE` is consistently set/forwarded to CMake in CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…f FileSystemFromUri, do not silently ignore non-empty options if underlying FS does not support them and use FileSystemFactoryOptions instead of std::vector<std::pair<std::string, std::any>> everywhere

… tests for FileSystemFromUriAndOptions and add a new one to test old behaviour

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Fix linting after rebase Add s3 fs module test for FromUriAndOptionsCredentials and refactor S3Options::FromUri and S3Options::FromUriAndOptions Add tests for FromUriAndOptions credentials Add S3RetryStrategy to options Add option for default_metadata Minor fix

raulcd added 4 commits June 8, 2026 10:57

apacheGH-46369: [C++] Add key-value pairs to the FileSystemFactory in…
7447a0f
…terface

Add test for extra options being passed and improve documentation
ce6aed2

Reject non-empty options, add tests to validate it and add missing pl…
426ad3b
…umbing for ARROW_S3_MODULE to CI

Fix CI and minor improvement on docstring
20069f4

CopilotAI review requested due to automatic review settings June 8, 2026 15:48

Copilotstarted reviewing on behalf of raulcd June 8, 2026 15:48 View session

raulcd changed the title ~~Gh 50126~~GH-50126: [C++] Make S3Filesystem consume key-value optionsJun 8, 2026

github-actionsBot added the awaiting committer review Awaiting committer review label Jun 8, 2026

CopilotAI reviewed Jun 8, 2026
View reviewed changes

raulcd mentioned this pull request Jun 9, 2026
GH-46369: [C++] Add key-value pairs to the FileSystemFactory interface and to S3Options #50044
Merged

raulcd added 6 commits June 9, 2026 12:47

Add test for typed option to validate std::any_cast cross DSO
ce9fd8a

Add missing new header file from commit :)
94142b2

Several review fixes, rename to FileSystemFromUriAndOptions instead o…
ddac9df
…f FileSystemFromUri, do not silently ignore non-empty options if underlying FS does not support them and use FileSystemFactoryOptions instead of std::vector<std::pair<std::string, std::any>> everywhere

A couple more review fixes to make copilot happy
eafbcd1

Re-add wrongly removed doxygen param due to copy&paste error
5c898f4

Remove unnecessary include, update a couple of error messages, re-org…
c9c887c
… tests for FileSystemFromUriAndOptions and add a new one to test old behaviour

raulcd force-pushed the GH-50126 branch from 0102f9b to 216be6dCompare June 9, 2026 15:50

github-actionsBot added the Component: C++ label Jun 9, 2026

CopilotAI review requested due to automatic review settings June 9, 2026 16:02

raulcd force-pushed the GH-50126 branch from 216be6d to c3e281eCompare June 9, 2026 16:02

Copilotstarted reviewing on behalf of raulcd June 9, 2026 16:02 View session

CopilotAI reviewed Jun 9, 2026
View reviewed changes

Comment threadcpp/src/arrow/filesystem/s3fs.cc Outdated
Comment threadcpp/src/arrow/filesystem/s3fs.cc Outdated
Comment threadcpp/src/arrow/filesystem/s3fs_module_test.cc

CopilotAI review requested due to automatic review settings June 11, 2026 11:25

raulcd force-pushed the GH-50126 branch from d9aa99a to beec31eCompare June 11, 2026 11:25

Copilotstarted reviewing on behalf of raulcd June 11, 2026 11:25 View session

CopilotAI reviewed Jun 11, 2026
View reviewed changes

Comment threadcpp/src/arrow/filesystem/filesystem.cc
Comment threadcpp/src/arrow/filesystem/s3fs.h
Comment threadcpp/src/arrow/filesystem/s3fs.cc Outdated

CopilotAI review requested due to automatic review settings June 12, 2026 07:18

Copilotstarted reviewing on behalf of raulcd June 12, 2026 07:18 View session

CopilotAI reviewed Jun 12, 2026
View reviewed changes

Comment threadcpp/src/arrow/filesystem/s3fs.h
Comment threadcpp/src/arrow/filesystem/s3fs.cc

raulcd force-pushed the GH-50126 branch from b1940e9 to 31ee32cCompare June 12, 2026 08:58

Navigation Menu

Uh oh!

GH-50126: [C++] Make S3Filesystem consume key-value options - #50128

GH-50126: [C++] Make S3Filesystem consume key-value options#50128
raulcd wants to merge 11 commits into
apache:mainfrom
raulcd:GH-50126

raulcd commented Jun 8, 2026•
edited by github-actions Bot
Loading

Uh oh!

github-actionsBot commented Jun 8, 2026

Uh oh!

CopilotAI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

raulcd commented Jun 8, 2026• edited by github-actions BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actionsBot commented Jun 8, 2026

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

raulcd commented Jun 8, 2026•
edited by github-actions Bot
Loading