GH-50697: [C++][FlightRPC] ODBC installer support fixes (previously GH-47877) - #49787
GH-50697: [C++][FlightRPC] ODBC installer support fixes (previously GH-47877)#49787alinaliBQ wants to merge 1 commit into
Conversation
|
|
e00ac1b to
a5484a2
Compare
|
cc @justing-bq Please help with code review comments on this PR going forward |
c42a315 to
847f5d1
Compare
9b4d76e to
3987150
Compare
alinaliBQ
left a comment
There was a problem hiding this comment.
@justing-bq Please have a look at the comments, thanks
|
@kou @lidavidm Please take a look when you have time, thanks! cc @justing-bq |
a94a2c9 to
0aff342
Compare
|
@justing-bq Has addressed all comments and the ODBC CIs are passing, the CI failures not from ODBC related changes. |
|
I hope that we use Could you split a fix for #50218 as a separated PR? |
eaf2333 to
1f56e49
Compare
|
@kou Thanks for putting up the PR #50288 for I just want to make sure I understand the ask, are you suggesting that:
I think it would be beneficial to include the ODBC Linux driver registration script. My initial thinking was that community could use CPack approach first, and then CPack approach can be phased out by Previous thread: #49766 (comment). |
Yes, we will work on that cc @justing-bq |
|
I'm suggesting 1. or 2. BTW, https://manpages.ubuntu.com/manpages/focal/man5/odbcinst.ini.5.html says that we should use We can use
We will not make the CPack based packages "offical" packages because it conflicts with existing deb/RPM packages. We should not provide non "official" packages to users. We can provide non "official" packages to only Apache Arrow developers. I think that it's not useful for you. So I suggest the
If you can't test, I can't help you because I don't know the expected behavior... |
56a1d7e to
5905363
Compare
Now using |
5905363 to
71d45f8
Compare
Hi @kou, our team can work on 2. Would it be okay with you if we only test the RPM installer on AlmaLinux 9 (i.e., only 1 Linux distro)? P.S. I'll be out of the office from July 14 and will be back on July 27. |
|
OK. Should we disable ODBC driver RPM for other distributions (AlmaLinux 8, CentOS Stream 9, AlmaLinux 10 and AmazonLinux 2023) because we can't test them? BTW, I've implemented auto driver installation: #50521 |
|
@kou Thank you for implementing the driver registration. Then our team will manually test the Linux RPM installer on AlmaLinux 9 to verify that the ODBC driver works as expected. If we encounter any minor issues, we'll investigate and address them where possible. 🙂
If you're referring to the CI, for now I don't think we need to disable the ODBC driver RPM for the other distributions, since the CI is passing on all of them. In theory, if the RPM installer works on AlmaLinux 9 out of the box, it should work on the other distributions. If our manual testing uncovers any issues on AlmaLinux 9, we can revisit whether to disable support for other distributions. |
|
|
5156eb3 to
e0e8fa8
Compare
- Add installer description to ReadMe - Fix `docs` on Windows to include docs Co-authored-by: justing-bq <justin.gossett@improving.com>
e0e8fa8 to
298c2a8
Compare
|
Could you close this PR and open a new PR from a new branch instead of reusing a PR for different change? |
Rationale for this change
GH-50697
What changes are included in this PR?
Are these changes tested?
Yes
Are there any user-facing changes?
No