bkietz commented on code in PR #41559:
URL: https://github.com/apache/arrow/pull/41559#discussion_r1966527385
##########
cpp/src/arrow/CMakeLists.txt:
##########
@@ -876,6 +876,18 @@ if(ARROW_FILESYSTEM)
foreach(ARROW_FILESYSTEM_TARGET ${ARROW_FILESYSTEM_TARGETS})
target_link_libraries(${ARROW_FILESYSTEM_TARGET} PRIVATE
${AWSSDK_LINK_LIBRARIES})
endforeach()
+
+ if(ARROW_S3_MODULE)
+ if(NOT ARROW_BUILD_SHARED)
+ message(FATAL_ERROR "ARROW_S3_MODULE without shared libarrow is not
supported")
+ endif()
+
+ add_library(arrow_s3fs MODULE filesystem/s3fs_module.cc
filesystem/s3fs.cc)
+ target_link_libraries(arrow_s3fs PRIVATE ${AWSSDK_LINK_LIBRARIES}
arrow_shared)
+ set_source_files_properties(filesystem/s3fs.cc filesystem/s3fs_module.cc
+ PROPERTIES SKIP_PRECOMPILE_HEADERS ON
+ SKIP_UNITY_BUILD_INCLUSION ON)
Review Comment:
Of course, another way to handle extended features of particular
`FileSystem`s is to add extra functions to its module. For example, let's say
that reading the resolved S3 region from a URI parameter as I described above
did not seem natural. Instead, we could add to s3fs_module.cc
```c++
extern "C" {
ARROW_FORCE_EXPORT char* arrow_filesystem_get_s3_region(const void* fs);
}
```
... then in python that function could be retrieved with `CDLL.__getattr__`.
I recommend avoiding this extra-C-functions approach if possible since it
introduces a de-facto ABI that would be a well hidden sharp edge in the
library. For resolving s3 region this could be as simple as "the caller is
responsible for freeing the returned string if it's not null". However if we
consider the more complex example of fully user programmable retry strategy,
extra-C-functions would involve
- negotiating the lifetime (both allocation/deallocation and
construction/destruction) of retry strategy objects
- documenting the retry strategy interface as ABI stable so that no
developers modify it (which would break an older libarrow_s3fs loaded by newer
libarrow)
- probably asserting that the major version of libarrow_${any}fs matches the
major version of libarrow which is loading it
- possibly also rewriting the retry strategy interface as a bundle of
function pointers as we do with other ABI objects
--
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]
