-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(be): sqlite support #3129
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
feat(be): sqlite support #3129
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 SQLite as a database backend alongside existing drivers.
- Introduces
DbDriverSQLite
in config, updates connection logic and CLI - Implements SQLite dialect in
SqlDb
, store factory, migrations, and CI workflows - Updates Docker wrapper and task logging/persistence to work with the new driver
Reviewed Changes
Copilot reviewed 16 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
util/config.go | Added DbDriverSQLite , extended dialect rules and connection string |
services/tasks/TaskRunner_logging.go | Cleaned up log flow, moved persistence into handleLogs |
services/tasks/TaskPool.go | Spun off log persistence into handleLogs goroutine |
go.mod | Added modernc.org/sqlite and related indirect modules |
deployment/docker/server/server-wrapper | Added SQLite env handling, ping logic and defaults |
db/sql/task.go | Trimmed trailing whitespace |
db/sql/migrations/v2.16.0.sqlite.sql | New migration for SQLite schema update |
db/sql/migrations/v2.16.0.sqlite.err.sql | Rollback script for SQLite migration |
db/sql/migrations/v2.15.1.sqlite.sql | Initial SQLite schema migration |
db/sql/SqlDb.go | Added CreateDb , GetDialect , and SQLite dialect setup |
db/factory/store.go | Updated store factory to return SQLite-backed SqlDb |
db/bolt/BoltDb.go | Added GetDialect method |
db/Store.go | Extended MigrationManager interface with GetDialect |
db/Migration.go | Passed dialect into GetMigrations , updated Rollback/Migrate |
cli/setup/setup.go | Added interactive prompts and scanSQLite helper |
.github/workflows/dev.yml | CI jobs for migrating and testing against SQLite |
Comments suppressed due to low confidence (4)
cli/setup/setup.go:226
- [nitpick] The prompt
db Hostname
is misleading when collecting a file path. Consider renaming it to something likedatabase file path
ordb filename
.
askValue("db Hostname", defaultDBPath, &conf.Hostname)
util/config.go:1047
- [nitpick] For SQLite, username and database name aren’t relevant. It may confuse users—consider printing only the file path (e.g.,
conf.SQLite.Hostname
).
fmt.Printf("SQLite %v@%v %v\n", conf.SQLite.GetUsername(), conf.SQLite.GetHostname(), conf.SQLite.GetDbName())
db/sql/SqlDb.go:40
- New SQLite support (factory method and dialect logic) isn’t covered by existing tests. Consider adding unit or integration tests for
CreateDb
,Connect
, and migrations against SQLite.
func CreateDb(dialect string) *SqlDb {
deployment/docker/server/server-wrapper:205
- The closing here-document delimiter
EOF
in the SQLite branch is indented, so it won’t terminate the here-doc. It must start at the beginning of the line (no leading spaces).
EOF
@@ -29,6 +29,7 @@ require ( | |||
golang.org/x/crypto v0.38.0 | |||
golang.org/x/oauth2 v0.30.0 | |||
gopkg.in/natefinch/lumberjack.v2 v2.2.1 | |||
modernc.org/sqlite v1.38.0 |
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.
There are many new indirect dependencies (e.g., go-humanize, bigfft, x/exp) that may not all be needed. Consider running go mod tidy
to prune unused modules.
modernc.org/sqlite v1.38.0 |
Copilot uses AI. Check for mistakes.
|
||
Dialect string `json:"dialect,omitempty" default:"bolt" rule:"^mysql|bolt|postgres$" env:"SEMAPHORE_DB_DIALECT"` | ||
Dialect string `json:"dialect,omitempty" default:"bolt" rule:"^mysql|bolt|postgres|sqlite$" env:"SEMAPHORE_DB_DIALECT"` |
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.
The regex in the rule
tag lacks grouping and could match unintended strings. It’s safer to use ^(mysql|bolt|postgres|sqlite)$
for exact matches.
Dialect string `json:"dialect,omitempty" default:"bolt" rule:"^mysql|bolt|postgres|sqlite$" env:"SEMAPHORE_DB_DIALECT"` | |
Dialect string `json:"dialect,omitempty" default:"bolt" rule:"^(mysql|bolt|postgres|sqlite)$" env:"SEMAPHORE_DB_DIALECT"` |
Copilot uses AI. Check for mistakes.
No description provided.