Merge lp:~roadmr/canonical-identity-provider/wrong-location-psql-config into lp:canonical-identity-provider/release

Proposed by Daniel Manrique on 2018-05-15
Status: Merged
Approved by: Daniel Manrique on 2018-05-16
Approved revision: 1633
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: lp:~roadmr/canonical-identity-provider/wrong-location-psql-config
Merge into: lp:canonical-identity-provider/release
Diff against target: 21 lines (+2/-2)
1 file modified
Makefile.db (+2/-2)
To merge this branch: bzr merge lp:~roadmr/canonical-identity-provider/wrong-location-psql-config
Reviewer Review Type Date Requested Status
Maximiliano Bertacchini 2018-05-15 Approve on 2018-05-16
Review via email: mp+345628@code.launchpad.net

Commit message

Fix location of custom Postgres config file.

The file should be in DATA_DIR, not in PGHOST directory. Thus,
the settings in this file were not really being applied.

Description of the change

Fix location of custom Postgres config file.

The file should be in DATA_DIR, not in PGHOST directory. Thus,
the settings in this file were not really being applied.

To post a comment you must log in.
Maximiliano Bertacchini (maxiberta) wrote :

The `setup-db` target will overwrite the default postgresql.conf file with the 3-line custom settings. Is that ok? Or should it *append* those 3 lines?

Daniel Manrique (roadmr) wrote :

I think it's meant to overwrite it, particularly since the default config file is mostly all commented out and has default values.

This is used for development only and I checked that all the tests pass (OTOH it wasn't any faster with the custom settings, beh). So it works but I'm happy to rescind this if you think it's weird (or maybe modify it so it does what you said and append to the existing file rather than clobbering it). No rush, let me know what you'd prefer :)

Maximiliano Bertacchini (maxiberta) wrote :

Looks like the original fabric script used to append the lines, but got lost in the translation into a Makefile. See https://bazaar.launchpad.net/~canonical-isd-hackers/canonical-identity-provider/trunk/revision/1250#fabtasks/database.py.

Daniel Manrique (roadmr) wrote :

+1 on archaeology!

Pushing a fix to append to the existing file, not clobbering it.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile.db'
2--- Makefile.db 2018-01-26 00:47:02 +0000
3+++ Makefile.db 2018-05-16 20:12:47 +0000
4@@ -4,7 +4,7 @@
5 PGUSER = $(shell $(SHELL_ENV) $(PYTHON) -c "from django.conf import settings; print(settings.DATABASES['default']['USER'])")
6 DATA_DIR = $(PGHOST)/data
7 LOG_FILE = $(PGHOST)/postgresql.log
8-CONF_FILE = $(PGHOST)/postgresql.conf
9+CONF_FILE = $(DATA_DIR)/postgresql.conf
10 PGBIN = /usr/lib/postgresql/9.3/bin
11 PGCTL = $(PGBIN)/pg_ctl
12 PGINIT = $(PGBIN)/initdb
13@@ -16,7 +16,7 @@
14 setup-db:
15 mkdir -p $(DATA_DIR)
16 $(PGINIT) -A trust -D $(DATA_DIR)
17- echo "fsync = off" > $(CONF_FILE)
18+ echo "fsync = off" >> $(CONF_FILE)
19 echo "standard_conforming_strings = off" >> $(CONF_FILE)
20 echo "escape_string_warning = off" >> $(CONF_FILE)
21 $(PGCTL) start -w -D $(DATA_DIR) -l $(LOG_FILE) -o "-F -k $(PGHOST) -h ''"