arrow icon indicating copy to clipboard operation
arrow copied to clipboard

GH-48312: [C++][FlightRPC] Standalone ODBC MSVC CI

Open alinaliBQ opened this issue 3 weeks ago • 10 comments

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

alinaliBQ avatar Dec 03 '25 00:12 alinaliBQ

:warning: GitHub issue #48278 has been automatically assigned in GitHub to PR creator.

github-actions[bot] avatar Dec 03 '25 00:12 github-actions[bot]

:warning: GitHub issue #48312 has been automatically assigned in GitHub to PR creator.

github-actions[bot] avatar Dec 03 '25 00:12 github-actions[bot]

I'll try to take a look but as an initial comment is the commit about moving from boost::optional to std::optional required 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

alinaliBQ avatar Dec 03 '25 18:12 alinaliBQ

I'll try to take a look but as an initial comment is the commit about moving from boost::optional to std::optional required 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

Do you want to submit this as another PR first?

lidavidm avatar Dec 03 '25 23:12 lidavidm

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.

alinaliBQ avatar Dec 04 '25 00:12 alinaliBQ

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:

alinaliBQ avatar Dec 04 '25 18:12 alinaliBQ

https://github.com/apache/arrow/pull/48323 is merged, and I have rebased and the std::optional commit is removed from this PR.

alinaliBQ avatar Dec 05 '25 00:12 alinaliBQ

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)

lidavidm avatar Dec 05 '25 00:12 lidavidm

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.

kou avatar Dec 05 '25 01:12 kou

@raulcd @kou @lidavidm The ODBC build under C++ Extra has passed. Please have another look, thank you!

alinaliBQ avatar Dec 12 '25 21:12 alinaliBQ

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?

pitrou avatar Dec 18 '25 09:12 pitrou

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).

raulcd avatar Dec 18 '25 10:12 raulcd

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

raulcd avatar Dec 18 '25 10:12 raulcd

Sorry, seems there is an issue with the caching, let me look into this issue today.

alinaliBQ avatar Dec 18 '25 18:12 alinaliBQ

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.