Skip to content

Conversation

@hoaidien
Copy link

@hoaidien hoaidien commented Aug 21, 2025

Add Connection Timeout and Keep-Alive Options to Flight SQL Client

Description

This PR adds four new optional configuration options to the Flight SQL client to allow fine-grained control over connection timeouts and keep-alive behavior. These options enable users to customize connection behavior based on their specific network conditions and requirements.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 📚 Documentation update

Changes Made

  • Added four new optional fields to ClientOptions: connectTimeout, timeout, keepAliveInterval, and keepAliveTimeout
  • Updated setup_client function to use the new timeout options instead of hardcoded 300-second values
  • All new options default to 20 seconds to maintain backward compatibility
  • Updated TypeScript definitions to include the new optional timeout fields
  • Added comprehensive documentation and examples for the new options

Breaking Changes

  • This PR introduces breaking changes

Breaking Changes Details:
None - all new options are optional and maintain backward compatibility.

Testing

  • All existing tests pass
  • New tests have been added for new functionality
  • Manual testing has been performed

Test Results:

> @presightio/[email protected] test
> ava

  ✔ returns native code version
  ─

  1 test passed

Documentation

  • Code changes are documented with appropriate comments
  • TypeScript definitions have been updated (if applicable)
  • README.md has been updated (if applicable)
  • Examples have been added/updated (if applicable)
  • API documentation has been updated (if applicable)

Dependencies

  • No new dependencies added

Performance Impact

  • No performance impact

The changes only affect connection setup and don't impact runtime query performance.

Platform Compatibility

  • Windows
  • macOS
  • Linux
  • Node.js version compatibility verified

Note: Build tested on Windows, should work on all platforms as it only changes Rust/NAPI bindings

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

Related Issues

Additional Notes

New Configuration Options

Option Type Default Description
connectTimeout number? 20 Maximum time in seconds to wait for initial connection
timeout number? 20 Maximum time in seconds to wait for individual requests
keepAliveInterval number? 300 Interval in seconds between keep-alive pings
keepAliveTimeout number? 20 Maximum time in seconds to wait for keep-alive responses

Usage Example

const client = await createFlightSqlClient({
  host: 'localhost',
  port: 8080,
  tls: false,
  headers: [],
  
  // New timeout options (all optional, in seconds)
  connectTimeout: 30,      // Connection timeout
  timeout: 60,             // Request timeout  
  keepAliveInterval: 120,  // Keep-alive ping interval
  keepAliveTimeout: 30,    // Keep-alive response timeout
});

Reviewer Notes

  • Please review the Rust field naming conventions (connect_timeout vs connectTimeout in TypeScript)
  • Verify that the default values (20 seconds) are appropriate for the use case
  • Check that the documentation is clear and comprehensive

For Maintainers:

  • This PR is ready for review
  • This PR requires design review
  • This PR requires security review
  • Version bump is required (patch/minor/major) - Minor version bump recommended for new features

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.

1 participant