-
Notifications
You must be signed in to change notification settings - Fork 144
test(csharp/src/Drivers): add Spice.ai test target #2961
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
Conversation
051f8ef
to
af18d6a
Compare
Shouldn't we test the driver directly instead of through C#? |
@lidavidm - thank you for review!
Good point. The reason the test was added based on the C# interop driver is to cover both:
Looking for recommendations/guidance here, will update as needed. |
@zeroshade what's your opinion here? |
I'm happy to have both types of tests; we also have C# tests for Flight interop with Denodo and Dremio. The bulk of the maintenance is likely to be in the Docker test fixture and that can be shared. |
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.
Thanks! I'm not the best person to comment on the workflow and test setup changes, but the C# changes look good to me, modulo a few nits.
csharp/test/Drivers/Interop/FlightSql/FlightSqlTestConfiguration.cs
Outdated
Show resolved
Hide resolved
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.
(Sorry for hijacking here but... @CurtHagenlocher could you check apache/arrow#46752 (comment) ? I want to proceed moving |
Add Spice.ai OSS test target for FlightSQL C# interop driver to test compatibility with arrow-rs FlightSQL implementation.
SpiceAI
environment type support (test data, example configuration)FligthSQL C# Interop
integration tests. This can be extended to include more tests targets via configuration files similar toci/configs/flightsql-spiceai.json
added in this PRExample test run: https://github.com/apache/arrow-adbc/actions/runs/15597223474/job/43930217202?pr=2961
Testing against ADBC helped to identify and fix:
GetDbSchemas
andGetTables
schemas to fully match the protocol arrow-rs#7638