GH-48068: [C++][FlightRPC] Linux ODBC: Configure Dremio instance to allow remote testing#49695
Conversation
|
|
7b6d340 to
4a892cf
Compare
alinaliBQ
left a comment
There was a problem hiding this comment.
Addressed code review comments
|
@kou @lidavidm All comments are addressed, please have another look. |
4a892cf to
3cf2376
Compare
3cf2376 to
3d18ebd
Compare
9ca9402 to
fa7ab48
Compare
There was a problem hiding this comment.
Pull request overview
This PR prepares the C++ Flight SQL ODBC CI environment to run remote tests against a Linux-only Dremio Docker instance by adding a Dremio service to the repo’s Docker Compose setup and automating initial Dremio bootstrap (user + test table) from CI.
Changes:
- Added a Dremio bootstrap script to create an admin user, obtain an auth token, and create
$scratch.ODBCTest. - Extended
compose.yamlwith adremioservice and injected an ODBC remote connection string into theubuntu-cpp-odbcservice. - Updated
cpp_extra.ymlto start Dremio and run the bootstrap script during the ODBC Linux job.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
cpp/src/arrow/flight/sql/odbc/tests/dremio/set_up_dremio_instance.sh |
New bootstrap script to provision Dremio for remote ODBC tests. |
compose.yaml |
Adds Dremio service + wires ODBC connection env var and dependency into the ODBC Docker job. |
.pre-commit-config.yaml |
Adds the new script to ShellCheck’s allowlist. |
.github/workflows/cpp_extra.yml |
Starts Dremio and runs the bootstrap script in the ODBC Linux workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
70abfae to
35c324f
Compare
35c324f to
ad2866f
Compare
91b8fa6 to
01d5339
Compare
01d5339 to
1e29550
Compare
kou
left a comment
There was a problem hiding this comment.
Could you update the PR description?
Is this Dremio instance used in Linux tests? How to confirm it?
| environment: | ||
| - DREMIO_JAVA_SERVER_EXTRA_OPTS=-Dsaffron.default.charset=UTF-8 -Dsaffron.default.nationalcharset=UTF-8 -Dsaffron.default.collation.name=UTF-8$$en_US | ||
| healthcheck: | ||
| test: curl --fail http://localhost:9047 || exit 1 |
There was a problem hiding this comment.
I don't believe it is necessary so I removed it.
I have updated the PR description to match the files updated. The Dremio instance is used in the ODBC Linux tests through the environment variable These test suites exclusively connect to the remote instance indicated in |
Authored by Justin Co-authored-by: justing-bq <justin.gossett@improving.com> ^ Conflicts: ^ .github/workflows/cpp_extra.yml
e6605f1 to
cc3b4fc
Compare
kou
left a comment
There was a problem hiding this comment.
+1
The Dremio instance is used in the ODBC Linux tests through the environment variable
ARROW_FLIGHT_SQL_ODBC_CONN. To confirm it, the ODBC test log should showFlightSQLODBCRemoteTestBase::*andFlightSQLOdbcV2RemoteTestBase::*tests.These test suites exclusively connect to the remote instance indicated in
ARROW_FLIGHT_SQL_ODBC_CONN, which in our case is the Dremio instance.
Thanks. We can't confirm it in CI because our CI log doesn't show executed test names, right?
https://github.com/apache/arrow/actions/runs/27574204620/job/81518244076?pr=49695#step:7:10585
+ ctest --label-regex unittest --output-on-failure --parallel 4 --repeat until-pass:3 --timeout 300
Test project /build/cpp
Start 1: arrow-array-test
Start 2: arrow-buffer-test
Start 3: arrow-extension-type-test
Start 4: arrow-misc-test
1/58 Test #3: arrow-extension-type-test ............. Passed 0.09 sec
Start 5: arrow-public-api-test
2/58 Test #2: arrow-buffer-test ..................... Passed 0.11 sec
Start 6: arrow-scalar-test
3/58 Test #4: arrow-misc-test ....................... Passed 0.12 sec
Start 7: arrow-type-test
4/58 Test #5: arrow-public-api-test ................. Passed 0.09 sec
Start 8: arrow-table-test
5/58 Test #7: arrow-type-test ....................... Passed 0.11 sec
Start 9: arrow-tensor-test
31/58 Test #34: arrow-compute-kernel-utility-test ..... Passed 0.08 sec
Start 35: arrow-canonical-extensions-test
32/58 Test #35: arrow-canonical-extensions-test ....... Passed 0.09 sec
Start 36: arrow-io-buffered-test
33/58 Test #33: arrow-compute-aggregate-test .......... Passed 0.60 sec
Start 37: arrow-io-compressed-test
34/58 Test #36: arrow-io-buffered-test ................ Passed 0.10 sec
Start 38: arrow-io-file-test
35/58 Test #31: arrow-compute-vector-selection-test ... Passed 1.00 sec
Start 39: arrow-io-memory-test
36/58 Test #38: arrow-io-file-test .................... Passed 0.16 sec
Start 40: arrow-utility-test
37/58 Test #40: arrow-utility-test .................... Passed 0.82 sec
Start 41: arrow-async-utility-test
38/58 Test #37: arrow-io-compressed-test .............. Passed 1.27 sec
Start 42: arrow-bit-utility-test
39/58 Test #39: arrow-io-memory-test .................. Passed 1.40 sec
Start 43: arrow-crc32-test
40/58 Test #43: arrow-crc32-test ...................... Passed 0.14 sec
Start 44: arrow-threading-utility-test
41/58 Test #42: arrow-bit-utility-test ................ Passed 1.85 sec
Start 45: arrow-json-integration-test
42/58 Test #45: arrow-json-integration-test ........... Passed 0.28 sec
Start 46: arrow-filesystem-test
43/58 Test #46: arrow-filesystem-test ................. Passed 1.27 sec
Start 47: arrow-flight-internals-test
44/58 Test #47: arrow-flight-internals-test ........... Passed 0.07 sec
Start 48: arrow-flight-test
45/58 Test #1: arrow-array-test ...................... Passed 9.82 sec
Start 49: arrow-flight-sql-test
46/58 Test #49: arrow-flight-sql-test ................. Passed 0.17 sec
Start 50: arrow-odbc-spi-impl-test
47/58 Test #50: arrow-odbc-spi-impl-test .............. Passed 0.08 sec
Start 51: arrow-flight-sql-odbc-test
48/58 Test #48: arrow-flight-test ..................... Passed 1.26 sec
Start 52: arrow-feather-test
49/58 Test #44: arrow-threading-utility-test .......... Passed 4.60 sec
Start 53: arrow-ipc-endianness-test
50/58 Test #53: arrow-ipc-endianness-test ............. Passed 0.15 sec
Start 54: arrow-ipc-message-internal-test
51/58 Test #54: arrow-ipc-message-internal-test ....... Passed 0.11 sec
Start 55: arrow-ipc-read-write-test
52/58 Test #52: arrow-feather-test .................... Passed 0.47 sec
Start 56: arrow-ipc-tensor-test
53/58 Test #56: arrow-ipc-tensor-test ................. Passed 0.10 sec
Start 57: arrow-json-test
54/58 Test #57: arrow-json-test ....................... Passed 0.35 sec
Start 58: arrow-telemetry-test
55/58 Test #41: arrow-async-utility-test .............. Passed 6.00 sec
56/58 Test #58: arrow-telemetry-test .................. Passed 0.11 sec
57/58 Test #55: arrow-ipc-read-write-test ............. Passed 2.16 sec
58/58 Test #51: arrow-flight-sql-odbc-test ............ Passed 9.48 sec
100% tests passed, 0 tests failed out of 58
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit cb625c5. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them. |
|
Yes, that's right. Since enabling tests against the Dremio instance doubles the number of test cases being run, we can perhaps check by the amount of time it takes to run the Before Dremio instance is enabled: After Dremio instance is enabled shows ~9 sec. |
Rationale for this change
The goal is to run remote tests against the Dremio instance on Ubuntu-latest runner, as Dremio docker instances are only available on Linux and they are not supported on macOS/Windows GitHub Actions.
What changes are included in this PR?
dremiotocompose.yaml.cpp/src/arrow/flight/sql/odbc/tests/dremio/set_up_dremio_instance.sh.ARROW_FLIGHT_SQL_ODBC_CONN.Note: This PR prepares the Dremio instance for remote testing, which is activated using
ARROW_FLIGHT_SQL_ODBC_CONN={..., HOST=dremio.Are these changes tested?
Tested in CI at local repository.
Are there any user-facing changes?
N/A