vikunja
/
vikunja
6
43
Fork 72
Code Issues 150 Pull Requests 19 Packages Releases 32 Activity

Add postgres support #135

Merged
konrad merged 26 commits from jtojnar/api:pgsql into master 2020-02-16 21:42:05 +00:00
Conversation 12 Commits 26 Files Changed 107 +25 -17
4 changed files with 25 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 5afced95c8 - Show all commits

2
docs/content/doc/setup/backups.md
View File

@ -42,7 +42,7 @@ You might be prompted for the password of the database user.
To restore it, simply pipe it back into the `psql` command:
{{< highlight bash >}}
mysql -U <user> -h <db-host> <database> < vikunja-backup.sql
psql -U <user> -h <db-host> <database> < vikunja-backup.sql
konrad commented 2020-02-15 10:21:58 +00:00
Outdated
Review
Copy Link

This should be psql.

This should be `psql`.
{{< /highlight >}}
For more information, please visit the [relevant PostgreSQL documentation](https://www.postgresql.org/docs/12/backup-dump.html).

3
docs/content/doc/setup/config.md
View File

@ -91,6 +91,9 @@ database:
maxidleconnections: 50
# The maximum lifetime of a single db connection in miliseconds.
maxconnectionlifetime: 10000
# Secure connection mode. Only used with postgres.
# (see https://pkg.go.dev/github.com/lib/pq?tab=doc#hdr-Connection_String_Parameters)
sslmode: disable
cache:
# If cache is enabled or not

2
pkg/config/config.go
View File

@ -55,6 +55,7 @@ const (
DatabaseMaxOpenConnections Key = `database.maxopenconnections`
DatabaseMaxIdleConnections Key = `database.maxidleconnections`
DatabaseMaxConnectionLifetime Key = `database.maxconnectionlifetime`
DatabaseSslMode Key = `database.sslmode`
CacheEnabled Key = `cache.enabled`
CacheType Key = `cache.type`
@ -181,6 +182,7 @@ func InitDefaultConfig() {
DatabaseMaxOpenConnections.setDefault(100)
DatabaseMaxIdleConnections.setDefault(50)
DatabaseMaxConnectionLifetime.setDefault(10000)
DatabaseSslMode.setDefault("disable")
// Cacher
CacheEnabled.setDefault(false)

35
pkg/db/db.go
View File

@ -120,26 +120,29 @@ func initMysqlEngine() (engine *xorm.Engine, err error) {
return
}
func postgresConnStrAppend(connStr *strings.Builder, name string, value string) {
if name != "" {
value = strings.ReplaceAll(value, "\\", "\\\\")
value = strings.ReplaceAll(value, "'", "\\'")
if connStr.Len() > 0 {
connStr.WriteString(" ")
}
connStr.WriteString(fmt.Sprintf("%s='%s'", name, value))
}
}
func initPostgresEngine() (engine *xorm.Engine, err error) {
jtojnar commented 2020-02-15 10:16:02 +00:00
Outdated
Review
Copy Link

This function is super ugly but I am not sure if there is a better way to do this. Maybe iterating over

params :=map[string]string{
    "user":      config.DatabaseUser.GetString(),
    "password":  config.DatabasePassword.GetString(),
    "host":      config.DatabaseHost.GetString(),
    "dbname":    config.DatabaseDatabase.GetString(),
}
This function is super ugly but I am not sure if there is a better way to do this. Maybe iterating over ``` params :=map[string]string{ "user": config.DatabaseUser.GetString(), "password": config.DatabasePassword.GetString(), "host": config.DatabaseHost.GetString(), "dbname": config.DatabaseDatabase.GetString(), } ```
konrad commented 2020-02-15 10:26:40 +00:00
Outdated
Review
Copy Link

A much cleaner way would be to use something like

connStr := fmt.Sprintf("user=%s password=% dbname=%s host=%s", config.DatabaseUser.GetString(), config.DatabasePassword.GetString(), config.DatabaseDatabase.GetString(), config.DatabaseHost.GetString())
A much cleaner way would be to use something like ```golang connStr := fmt.Sprintf("user=%s password=% dbname=%s host=%s", config.DatabaseUser.GetString(), config.DatabasePassword.GetString(), config.DatabaseDatabase.GetString(), config.DatabaseHost.GetString()) ```
jtojnar commented 2020-02-15 10:29:56 +00:00
Outdated
Review
Copy Link

Well, we still want to be able to not pass some of those keys. For example, I connect to the database via socket so username and password does not make sense.

Well, we still want to be able to not pass some of those keys. For example, I connect to the database via socket so username and password does not make sense.
konrad commented 2020-02-15 11:18:20 +00:00
Outdated
Review
Copy Link

I think we could use something like this: ad1b6d439f/modules/setting/database.go (L152-L162)

I think we could use something like this: https://github.com/go-gitea/gitea/blob/ad1b6d439fe0e0875e54227e0bc23a74411f490e/modules/setting/database.go#L152-L162
var connStr strings.Builder
// https://pkg.go.dev/github.com/lib/pq?tab=doc#hdr-Connection_String_Parameters
postgresConnStrAppend(&connStr, "user", config.DatabaseUser.GetString())
postgresConnStrAppend(&connStr, "password", config.DatabasePassword.GetString())
postgresConnStrAppend(&connStr, "host", config.DatabaseHost.GetString())
postgresConnStrAppend(&connStr, "dbname", config.DatabaseDatabase.GetString())
params := map[string]string{
"user": config.DatabaseUser.GetString(),
"password": config.DatabasePassword.GetString(),
"host": config.DatabaseHost.GetString(),
"dbname": config.DatabaseDatabase.GetString(),
"sslmode": config.DatabaseSslMode.GetString(),
}
for name, value := range params {
if name != "" {
value = strings.ReplaceAll(value, "\\", "\\\\")
value = strings.ReplaceAll(value, "'", "\\'")
jtojnar commented 2020-02-15 07:50:13 +00:00
Outdated
Review
Copy Link

We will probably need to allow passing the rest of the options. The Go library recommended by xorm decided to stray from libpq’s defaults and forces sslmode=required, which fails on my laptop where I do not have TLS set up.

Or maybe we can just accept raw connection string?

We will probably need to allow passing the rest of the options. The Go library recommended by xorm decided to stray from libpq’s defaults and forces `sslmode=required`, which fails on my laptop where I do not have TLS set up. Or maybe we can just accept raw connection string?
konrad commented 2020-02-15 10:10:48 +00:00
Outdated
Review
Copy Link

I'd say we probably should add a setting for sslmode.

I'd say we probably should add a setting for `sslmode`.
if connStr.Len() > 0 {
connStr.WriteString(" ")
}
connStr.WriteString(fmt.Sprintf("%s='%s'", name, value))
jtojnar commented 2020-02-16 13:48:03 +00:00
Outdated
Review
Copy Link

Why not just use the attr=name format instead of URI format? Or use this one for both UNIX socket and TCP connection? This only needs to be used for UNIX sockets because URI hostnames cannot contain / but the other way around is fine.

Why not just use the `attr=name` format instead of URI format? Or use this one for both UNIX socket and TCP connection? This only needs to be used for UNIX sockets because URI hostnames cannot contain `/` but the other way around is fine.
konrad commented 2020-02-16 14:07:22 +00:00
Outdated
Review
Copy Link

Good point. What do think would be better suited, using the same for both or using the attr=name format?

Good point. What do think would be better suited, using the same for both or using the `attr=name` format?
jtojnar commented 2020-02-16 16:54:48 +00:00
Outdated
Review
Copy Link

I think attr=%s is clearer.

I think `attr=%s` is clearer.
konrad commented 2020-02-16 16:56:38 +00:00
Outdated
Review
Copy Link

Then lets change it.

Then lets change it.
}
}
engine, err = xorm.NewEngine("postgres", connStr.String())
if err != nil {
jtojnar commented 2020-02-15 07:16:34 +00:00
Outdated
Review
Copy Link

I am not sure if these flags are needed.

I am not sure if these flags are needed.
jtojnar commented 2020-02-15 07:51:29 +00:00
Outdated
Review
Copy Link

I am not sure if these flags are needed.

I am not sure if these flags are needed.
konrad commented 2020-02-15 10:08:36 +00:00
Outdated
Review
Copy Link

I'd say yes, since the max number of open connections can be a limiting factor for performance.

I'd say yes, since the max number of open connections can be a limiting factor for performance.