-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jdbc enable httpfs #2442
Jdbc enable httpfs #2442
Conversation
@Mytherin first time contributor here. Looks like the last run was all good minus an intermittent test failure re: python and arrow file download parsing. Merged master in again to trigger a new run. Anything else I need to take care of please let me know! |
Thanks for the PR! This looks good to me in principle. Perhaps we should use the Otherwise I can potentially see linking issues when deploying the |
Thanks for the tips, still getting familiar with the code base. Will take a crack at making those changes this week. |
@@ -215,6 +215,8 @@ jobs: | |||
BUILD_FTS: 1 | |||
BUILD_REST: 1 | |||
BUILD_JDBC: 1 | |||
BUILD_HTTPFS: 1 | |||
OPENSSL_ROOT_DIR: /usr/local/opt/openssl/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not going to be the correct include path for OpenSSL on most platforms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is this is only relevant to the xcode-release
build job on macos. The linux-release-64 did not require additional settings as I believe the correct reference is already set in the environment.
@Mytherin I did update to use extension helper and cmake macros but you are spot on that it has the same linking issues while running on systems without libssl.so.1.0.0 installed. Not 100% on how to proceed. Perhaps now that it uses the extension helper it can take advantage of the fix in the future. I am not sure if statically linking libssl is a good alternative but it may benefit those who want to run on systems that have libssl.so.1.1 vs libssl.so.1.0.0. Thoughts? |
This will be superseded by #2569 |
Initial take on using BUILD_HTTPFS to also include httpfs extension when building tools/jdbc shared object libs for both osx and linux amd64.