Copilot commented on code in PR #50044:
URL: https://github.com/apache/arrow/pull/50044#discussion_r3403740850


##########
cpp/src/arrow/filesystem/filesystem.h:
##########
@@ -357,13 +359,44 @@ class ARROW_EXPORT FileSystem
   bool default_async_is_sync_ = true;
 };
 
+using FileSystemFactoryOptions = std::vector<std::pair<std::string, std::any>>;
+
 struct FileSystemFactory {
   std::function<Result<std::shared_ptr<FileSystem>>(
-      const Uri& uri, const io::IOContext& io_context, std::string* out_path)>
+      const Uri& uri, const FileSystemFactoryOptions& options,
+      const io::IOContext& io_context, std::string* out_path)>
       function;
   std::string_view file;
   int line;
 
+  /// Construct from an options-aware factory function.
+  FileSystemFactory(std::function<Result<std::shared_ptr<FileSystem>>(
+                        const Uri&, const FileSystemFactoryOptions&, const 
io::IOContext&,
+                        std::string*)>
+                        fn,
+                    std::string_view file, int line)
+      : function(std::move(fn)), file(file), line(line) {}
+
+  /// Construct from a non-options aware factory function maintaining source 
compatibility
+  /// with existing factories.
+  FileSystemFactory(std::function<Result<std::shared_ptr<FileSystem>>(
+                        const Uri&, const io::IOContext&, std::string*)>
+                        fn,
+                    std::string_view file, int line)
+      : function([fn = std::move(fn)](
+                     const Uri& uri, const FileSystemFactoryOptions& options,
+                     const io::IOContext& ctx,
+                     std::string* out_path) -> 
Result<std::shared_ptr<FileSystem>> {
+          if (!options.empty()) {
+            return Status::NotImplemented(
+                "Filesystem factory does not support additional options, got ",
+                options.size(), " option(s)");
+          }
+          return fn(uri, ctx, out_path);

Review Comment:
   The NotImplemented error produced by the legacy (non-options-aware) factory 
wrapper doesn’t indicate which URI scheme rejected the options, unlike the 
NotImplemented path in `FileSystemFromUriReal()`. Including `uri.scheme()` here 
would make diagnostics clearer when multiple factories are registered.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to