Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

#90: add connection params to psql init #115

Merged
merged 1 commit into from
Sep 20, 2017
Merged

Conversation

liortamari
Copy link

@liortamari liortamari commented Sep 20, 2017

This change is Reviewable

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 51.696% when pulling d70cb88 on feature/liort_db_params into cb0edca on dev.

@liortamari liortamari merged commit 6c33a0e into dev Sep 20, 2017
@liortamari liortamari deleted the feature/liort_db_params branch September 20, 2017 12:01
@shay-berman
Copy link
Contributor

Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions.


database/init.go, line 28 at r1 (raw file):

    KeySqlitePath = "UBIQUITY_DB_SQLITE_PATH"
    KeyPsqlUser = "UBIQUITY_DB_USER"
    KeyPsqlPassword = "UBIQUITY_DB_PASSWORD"

next time, please try to commit it on different ticket, because its different subject related to PG security.


database/init.go, line 45 at r1 (raw file):

    psqlDbName := os.Getenv(KeyPsqlDbName)
    if psqlDbName == "" {
        psqlDbName = "postgres"

Add them as const like PSGLDbNameDefault="postgres" also for the user


database/init.go, line 56 at r1 (raw file):

    psqlPort := os.Getenv(KeyPsqlPort)
    if psqlPort != "" {
        str += " port=" + psqlPort

is there any validation check on the port to validate its a number? or we just fail with panic?


database/init.go, line 67 at r1 (raw file):

func InitPostgres(hostname string) func() {
    defer logs.GetLogger().Trace(logs.DEBUG)()
    return initConnectionFactory(&postgresFactory{psql: GetPsqlConnectionParams(hostname) + " " + GetPsqlSslParams()})

is there any special reason why you do it as long strings with key=val rather then a map?


Comments from Reviewable

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants