GH-48312: [C++][FlightRPC] Standalone ODBC MSVC CI
Rationale for this change
https://github.com/apache/arrow/issues/48312
What changes are included in this PR?
- Add new ODBC workflow for Windows MSVC
- Added build fixes to enable build on MSVC CI
Are these changes tested?
Tested in CI
Are there any user-facing changes?
N/A
- GitHub Issue: #48312
:warning: GitHub issue #48278 has been automatically assigned in GitHub to PR creator.
:warning: GitHub issue #48312 has been automatically assigned in GitHub to PR creator.
I'll try to take a look but as an initial comment is the commit about moving from
boost::optionaltostd::optionalrequired for this PR? a696be3
Yes I think so, previously when I was testing my changes on a different branch, the build was failing due to mixed usages of boost::optional and std::optional. Having this commit resolved the build failures
I'll try to take a look but as an initial comment is the commit about moving from
boost::optionaltostd::optionalrequired for this PR? a696be3Yes I think so, previously when I was testing my changes on a different branch, the build was failing due to mixed usages of
boost::optionalandstd::optional. Having this commit resolved the build failures
Do you want to submit this as another PR first?
Do you want to submit this as another PR first?
Sure, I have extracted the std::optional change in https://github.com/apache/arrow/pull/48323.
Why do we need separated workflow?
Good question @kou. I took @lidavidm's suggestion to have a separate workflow for ODBC and make it a nightly build so it runs on a schedule. I think, 1) having ODBC in its own workflow that only gets triggered by ODBC changes will prevent workflow failures on main branch for non-ODBC changes, to minimize impact on others' branches. 2) We need ODBC in the release build, currently MSVC CI is in debug mode by default. 3) When enabling ODBC build in release mode, we ran into errors with other non-ODBC Arrow components, hopefully having the CI only build ODBC and its dependencies will minimize that.
@lidavidm Feel free to add any reasoning from your side! :slightly_smiling_face:
https://github.com/apache/arrow/pull/48323 is merged, and I have rebased and the std::optional commit is removed from this PR.
Also because the goal of the ODBC driver is to produce an MSI that can be included as part of the release and distributed, and so we need some workflow that can generate that (now possibly this should be a crossbow job instead, though)
OK. Then could you cpp_extra.yml instead of separated workflow?
We can run jobs in cpp_extra.yml by adding CI: Extra: C++ label. And cpp_extra.yml is executed daily.
@raulcd @kou @lidavidm The ODBC build under C++ Extra has passed. Please have another look, thank you!
These builds always take very long (around 1h40), 1) are we sure vcpkg cacking is working? 2) can we stop building large unused dependencies such as aws-sdk-cpp and google-cloud-cpp?
Just noticed this too on my PR, caching does not seem to be working, there seems to have some credential issues:
WARNING: Your request could not be authenticated by the GitHub Packages service. Please ensure your access token is valid and has the appropriate scopes configured.
Forbidden https://nuget.pkg.github.com/apache/ 7052ms
Response status code does not indicate success: 403 (Forbidden).
System.Net.Http.HttpRequestException: Response status code does not indicate success: 403 (Forbidden).
I can also see some timeouts and I've seen some problems on other jobs connecting to github to download packages so I am unsure whether the problem is that GitHub is having issues or if there's really a problem with credentials:
Starting submission of google-cloud-cpp[core,rest-common,storage]:[email protected] to 1 binary cache(s) in the background
Elapsed time to handle google-cloud-cpp:x64-windows: 5 min
google-cloud-cpp:x64-windows package ABI: 8e20db0777ec996c988874fe6425f50651387f6a355a4326a6678c64f235d355
error: "C:\ProgramData\Chocolatey\bin\nuget.exe" push -ForceEnglishOutput -Verbosity detailed -NonInteractive "D:\a\arrow\arrow\vcpkg\buildtrees\crc32c_x64-windows.1.1.2-vcpkg1b2e2cd4d288cef27d01ba4088df517007c115d52a20222fe4f7aa2621a85739.nupkg" -Timeout 100 -Source GitHub failed with exit code 1
NuGet Version: 7.0.1.1
Sorry, seems there is an issue with the caching, let me look into this issue today.
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit c58bfe5e81bf828370020c1dae3b4949a8889de1.
There were no benchmark performance regressions. 🎉
The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.