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

Proposed by Daniel Manrique
Status: Merged
Approved by: Daniel Manrique
Approved revision: no longer in the source branch.
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 Approve
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.
Revision history for this message
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?

Revision history for this message
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 :)

Revision history for this message
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.

Revision history for this message
Daniel Manrique (roadmr) wrote :

+1 on archaeology!

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

Revision history for this message
Maximiliano Bertacchini (maxiberta) :
review: Approve
Revision history for this message
Otto Co-Pilot (otto-copilot) wrote :

Preview Diff

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