added shared connection pool

[LinoGiger]

No description provided.

[claude]

PR Review: Added Shared Connection Pool

Summary

This PR adds the ability to share an OpenAPIService instance between multiple RapidataClient instances, enabling connection pool reuse. This is a valuable feature for preventing resource exhaustion when creating multiple clients.

Code Quality & Best Practices

✅ Strengths

1. Clean implementation: The changes are minimal and non-breaking, properly using an optional parameter
2. Good documentation: The docstring clearly explains the purpose and usage of the openapi_service parameter
3. Consistent with project style: Uses from __future__ import annotations and proper type hints as per CLAUDE.md guidelines
4. Proper encapsulation: Exposes openapi_service as a read-only property, preventing accidental modifications

🔍 Observations

1. Export in __init__.py: The PR exports OpenAPIService from the main package. While this makes it accessible for sharing, it exposes an internal implementation detail that wasn't previously part of the public API. Consider whether this is intentional or if documentation should guide users on this pattern.

2. Environment mismatch handling: When reusing an openapi_service, the code checks openapi_service.environment != "rapidata.ai" to disable OTLP logging. However, there's no validation that the passed openapi_service has compatible settings (environment, oauth_scope, cert_path, etc.) with any other parameters provided.

Potential Issues

⚠️ Parameter Validation

When openapi_service is provided, the other authentication parameters (client_id, client_secret, environment, oauth_scope, cert_path, token, leeway) are ignored. This could lead to confusion if users provide both. Consider adding validation warnings.

🔒 Thread Safety Consideration

The connection pool (httpx Limits set to 200 max connections in rest.py:243) is shared when reusing an OpenAPIService. This is generally fine for httpx clients, but users should be aware that:

• The shared session handles token refresh internally (OAuth2Client)
• Multiple clients using the same service will share the same token refresh mechanism
• This is likely the desired behavior, but should be documented

Performance Considerations

✅ Positive impact: The main benefit of this PR is realized. Sharing connection pools prevents:

• Opening excessive connections when creating multiple clients
• Resource exhaustion from too many file descriptors
• Overhead of repeated TLS handshakes

Security Concerns

⚠️ Token Sharing

When sharing an OpenAPIService, all RapidataClient instances share the same OAuth token and credentials. This means:

• All clients will authenticate as the same user/service
• Token refresh affects all clients simultaneously
• Credential resets affect all clients

This is likely intentional, but should be documented clearly.

Test Coverage

⚠️ Missing Tests

No tests were added for this functionality. Consider adding tests for:

1. Creating multiple clients with a shared openapi_service
2. Verifying that the openapi_service property returns the correct instance
3. Testing that shared clients properly reuse connections
4. Validating behavior when both openapi_service and auth parameters are provided

Recommendations

1. Add validation: Warn users if they provide both openapi_service and conflicting auth parameters
2. Add tests: Cover the new functionality with unit tests
3. Documentation: Consider adding an example in the docstring or README showing the recommended pattern for creating multiple clients
4. Type checking: Run uv run pyright src/rapidata/rapidata_client to ensure no type errors were introduced (as per CLAUDE.md guidelines)

Conclusion

This is a solid addition that solves a real problem (connection pool exhaustion) with minimal changes. The implementation is clean and follows project conventions. With the addition of parameter validation warnings and test coverage, this would be production-ready.

Recommendation: Approve with suggested improvements (non-blocking)