Skip to content

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

Merged
merged 5 commits into from
Jun 30, 2025

Conversation

sgrebnov
Copy link
Member

@sgrebnov sgrebnov commented Jun 11, 2025

Add Spice.ai OSS test target for FlightSQL C# interop driver to test compatibility with arrow-rs FlightSQL implementation.

  • Interop/FlightSql: add SpiceAI environment type support (test data, example configuration)
  • ci: spin up Spice test server using docker compose
  • ci: extend integration tests to include FligthSQL C# Interop integration tests. This can be extended to include more tests targets via configuration files similar to ci/configs/flightsql-spiceai.json added in this PR

Example test run: https://github.com/apache/arrow-adbc/actions/runs/15597223474/job/43930217202?pr=2961

Testing against ADBC helped to identify and fix:

@github-actions github-actions bot added this to the ADBC Libraries 19 milestone Jun 11, 2025
@sgrebnov sgrebnov changed the title Add Spice.ai test target for FlightSQL C# interop driver feat(flightsql/csharp): add Spice.ai integration test target Jun 11, 2025
@sgrebnov sgrebnov force-pushed the sgrebnov/spiceai-test-target branch from 051f8ef to af18d6a Compare June 11, 2025 22:39
@sgrebnov sgrebnov changed the title feat(flightsql/csharp): add Spice.ai integration test target test(csharp/src/Drivers): add Spice.ai test target Jun 11, 2025
@lidavidm
Copy link
Member

Shouldn't we test the driver directly instead of through C#?

@sgrebnov
Copy link
Member Author

sgrebnov commented Jun 12, 2025

@lidavidm - thank you for review!

Shouldn't we test the driver directly instead of through C#?

Good point. The reason the test was added based on the C# interop driver is to cover both:

  • general ADBC compatibility (based on internal usage of the Go ADBC driver)
  • full support/integration with clients based on the C# interop driver (important)

Looking for recommendations/guidance here, will update as needed.

@lidavidm
Copy link
Member

@zeroshade what's your opinion here?

@CurtHagenlocher
Copy link
Contributor

Shouldn't we test the driver directly instead of through C#?

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.

Copy link
Contributor

@CurtHagenlocher CurtHagenlocher left a 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.

Copy link
Contributor

@CurtHagenlocher CurtHagenlocher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This looks good to me, but perhaps @lidavidm or @kou can take a look at the CI changes?

@kou
Copy link
Member

kou commented Jun 23, 2025

(Sorry for hijacking here but... @CurtHagenlocher could you check apache/arrow#46752 (comment) ? I want to proceed moving csharp/ in apache/arrow to apache/arrow-dotnet.)

@sgrebnov sgrebnov requested a review from lidavidm June 30, 2025 22:59
@lidavidm lidavidm merged commit ca929ee into apache:main Jun 30, 2025
64 of 68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants