Add support for Multiple DB Drivers in OGA Service#269
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant architectural enhancement to the OGA application by implementing a generic database connection mechanism. This change abstracts the underlying database technology, enabling the application to seamlessly switch between SQLite and PostgreSQL. The primary impact is increased flexibility for deployment scenarios, allowing users to choose a database solution that best fits their operational needs without requiring code modifications. The update involves changes across configuration, dependency management, and core database initialization logic. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request implements support for PostgreSQL as a database backend alongside the existing SQLite, which is a great enhancement. The changes are spread across configuration files, Docker setup, and the Go application code to allow for a generic database connection. My review focuses on security and maintainability aspects introduced by these changes. I've identified a critical security issue with hardcoded credentials in the Dockerfile, and other high/medium severity issues related to potential credential leakage in logs, weak default passwords, and code duplication in configuration. Please find detailed comments below.
4c21311 to
6dbb91b
Compare
|
@ginaxu1 can you move the new connectors in to a separate folder inside And don't we need to update the root |
68beb52 to
ffb8a21
Compare
|
@ginaxu1 did you tested with postgres configuration? |
e98d7fb to
3f44f98
Compare
3f44f98 to
54d4f2a
Compare
54d4f2a to
f2c0887
Compare
Fix docker compose run
Closes #246
Closes #250
Summary
Introduce a driver-agnostic DB layer for the OGA service per the interface + factory pattern. Add official support for PostgreSQL while maintaining SQLite as the default option for local development and backward compatibility, ensuring that the core storage logic never needs to be modified when adding new DB engines in the future
Changes
OGA Backend
oga/internal/connector.go: introduce theDBConnectorinterface to abstract GORM connection logicoga/internal/sqlite_connector.go&oga/internal/postgres_connector.go: Created dedicated files to isolate driver-specific dependencies and centralize DSN (Data Source Name) construction for each database typeoga/internal/factory.go: implementNewDBConnector, a factory function that handles the switch mechanism to instantiate the correct driver based on the environment configurationoga/internal/store.go: refactorNewApplicationStoreto completely decouple it from specific GORM drivers. It now relies entirely on the factory and interface, removing all hardcoded driver importsoga/internal/config.go: add new configuration fields to support database driver selection (OGA_DB_DRIVER) and PostgreSQL-specific connection parameters (Host, Port, User, Password, DB Name, and SSL Mode).oga/go.mod&oga/go.sum: add PostgreSQL driver dependenciesInfrastructure & Scripts
Dockerfile: update to include the new database environment variables, ensuring they are available within the containerized environmentdocker-compose.yml: add database configuration blocks for all three OGA service instances (nsw-oga-npqs,nsw-oga-fcau, andnsw-oga-ird) to allow them to connect to external or containerized PostgreSQL instancesstart-dev.sh: update the local development script to pass through the new database environment variables to the OGA processesstart-docker.sh: update the startup summary output to explicitly display which database driver is currently active for better visibility during troubleshooting.Configuration
oga/.env.example&.env.example: update templates with detailed sections for PostgreSQL configuration, including default values for local setup (e.g.,sslmode=disable)