Skip to content

Allow queries without authentication if no API key is provided - #70

Merged
ewgenius merged 8 commits into
trunkfrom
evgenii/2026-01-06/fix-non-authenticated-client
Jan 6, 2026
Merged

Allow queries without authentication if no API key is provided#70
ewgenius merged 8 commits into
trunkfrom
evgenii/2026-01-06/fix-non-authenticated-client

Conversation

@ewgenius

@ewgenius ewgenius commented Jan 6, 2026

Copy link
Copy Markdown
Contributor

🗣 Description

This pull request improves authentication handling for both cloud and local Spice runtimes, refactors tests and benchmarks to skip cloud tests if the API key is not set, and introduces more robust test coverage for authenticated and unauthenticated queries. The changes also update parameterized query tests to use the taxi_trips dataset instead of the previous TPC-H dataset, making the tests more relevant to current data sources.

Tests updated to use https://spice.ai/spiceai/quickstart Spice Cloud app

Authentication Handling Improvements

  • Updated all tests and benchmarks to skip cloud authentication tests if SPICE_API_KEY is not set or empty, instead of using a hardcoded test API key. This ensures sensitive credentials are handled securely and prevents accidental test execution against the cloud without proper authentication. [1] [2] [3] [4] [5] [6] [7]

  • Modified the internal query logic in query.go to only perform authentication if both appId and apiKey are provided, allowing unauthenticated queries against local runtimes.

Test Coverage Enhancements

  • Added new tests in query_test.go to explicitly verify that SQL queries work both with and without authentication, including readiness and health checks for local and cloud Spice runtimes.

Parameterized Query Test Updates

  • Refactored parameterized query tests in adbc_test.go to use the taxi_trips dataset instead of TPC-H, updating field names, query parameters, and validation logic to match the new schema. This makes the tests more relevant and easier to maintain. [1] [2] [3]

Code Clean-up

  • Removed the hardcoded TEST_API_KEY constant from query_test.go and replaced all usages with environment variable checks.

These changes collectively improve security, reliability, and maintainability of tests and benchmarks for both local and cloud environments.

Add tests for Sql method with and without authentication

These tests verify that queries work both with and without API key
against local and cloud Spice runtimes.
@ewgenius ewgenius self-assigned this Jan 6, 2026
@ewgenius ewgenius added the bug Something isn't working label Jan 6, 2026
@ewgenius
ewgenius requested a review from Copilot January 6, 2026 06:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request enhances authentication handling in the query logic to support optional authentication and adds comprehensive test coverage for both authenticated and unauthenticated query scenarios. The changes enable SQL queries to execute without authentication when no API credentials are provided, which is essential for local runtime environments.

Key Changes:

  • Modified queryInternal function to conditionally authenticate only when both appId and apiKey are provided
  • Added TestSqlWithoutAuth to verify SQL query execution against local Spice runtime without authentication
  • Added TestSqlWithAuth to verify SQL query execution against Spice Cloud with authentication

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
query.go Updated authentication logic to only authenticate when credentials are provided, using the original context when credentials are absent
query_test.go Added comprehensive test coverage for authenticated and unauthenticated SQL queries with health checks, readiness validation, and dataset queries

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread query_test.go Outdated
Comment thread query_test.go Outdated
@ewgenius
ewgenius merged commit 45f32c4 into trunk Jan 6, 2026
9 checks passed
@ewgenius
ewgenius deleted the evgenii/2026-01-06/fix-non-authenticated-client branch January 6, 2026 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants