-
Notifications
You must be signed in to change notification settings - Fork 11
support plain DataSource #268
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
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.
Pull request overview
This PR adds support for plain DataSource interfaces instead of requiring HikariDataSource throughout the codebase. The change enables using any JDBC DataSource implementation, such as PGSimpleDataSource, while maintaining backward compatibility with Hikari-specific features when a HikariDataSource is provided.
Changes:
- Updated all DAO classes and SystemDatabase to use the generic
DataSourceinterface instead ofHikariDataSource - Removed redundant
isClosed()checks that are specific to HikariDataSource - Added runtime type checks to conditionally access Hikari-specific features like pool management
- Updated DBOSConfig to accept any DataSource implementation
- Added test coverage for using PGSimpleDataSource alongside existing HikariDataSource tests
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java | Changed dataSource field and withDataSource method to use generic DataSource interface |
| transact/src/main/java/dev/dbos/transact/database/SystemDatabase.java | Updated to use DataSource interface, added conditional casting for Hikari-specific operations |
| transact/src/main/java/dev/dbos/transact/database/WorkflowDAO.java | Changed to use DataSource and removed HikariDataSource-specific isClosed() checks |
| transact/src/main/java/dev/dbos/transact/database/StepsDAO.java | Updated to use DataSource interface and removed isClosed() checks |
| transact/src/main/java/dev/dbos/transact/database/QueuesDAO.java | Changed to use DataSource and removed isClosed() checks |
| transact/src/main/java/dev/dbos/transact/database/NotificationsDAO.java | Updated to use DataSource interface and removed isClosed() checks |
| transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java | Changed runMigrations method parameter to accept generic DataSource |
| transact/src/test/java/dev/dbos/transact/database/DBTestAccess.java | Updated return type to Optional to handle non-Hikari data sources |
| transact/src/test/java/dev/dbos/transact/config/ConfigTest.java | Added new test for PGSimpleDataSource and updated existing test to handle Optional |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
transact/src/main/java/dev/dbos/transact/database/SystemDatabase.java
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.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Does this fix #136? |
Not really. I mean if you provide a non Hikari data source, then you won't get Hikari data source logs, but otherwise doesn't change anything related to logging |
DBOS uses
HikariDataSourceif a data source is not provided. This PR updated sysdb + dbos config to support baseDataSource(i.e. non Hikari data sources)Also added DBOS.registerQueues to register more than one queue at a time