Merge ~doismellburning/launchpad:bail-on-fail-on-db-setup into launchpad:master

Proposed by Kristian Glass
Status: Merged
Approved by: Kristian Glass
Approved revision: de60cbfe981f9a466cae7495a69c3651103cfddb
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~doismellburning/launchpad:bail-on-fail-on-db-setup
Merge into: launchpad:master
Diff against target: 33 lines (+4/-4)
1 file modified
utilities/launchpad-database-setup (+4/-4)
Reviewer Review Type Date Requested Status
Ioana Lasc Approve
Colin Watson Approve
Review via email: mp+387647@code.launchpad.net

Commit message

Exit on failure in launchpad-database-setup

Let's avoid:

    Creating postgresql user runner
    createuser: could not connect to database postgres

    Looks like everything went ok.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

This needs other changes further down the script; anywhere that runs a command and then tests $? is going to need to be adjusted to work with set -e.

review: Needs Fixing
Revision history for this message
Kristian Glass (doismellburning) wrote :

Good point, thanks - I don't think I love these variable names, but they should do the job!

Revision history for this message
Colin Watson (cjwatson) wrote :

I normally just use "code" or something similarly not-very-nymous for this sort of thing, but this is fine.

If you prefer, you could also consider this style, which may be neater in this case:

  if sudo grep -qs "^auto" /etc/postgresql/$try_pgversion/main/start.conf; then
    pgversion="$try_pgversion"
    break
  fi

and:

  if ! sudo grep -q "port.*5432" /etc/postgresql/$pgversion/main/postgresql.conf; then
    echo "..."
  fi

review: Approve
Revision history for this message
Ioana Lasc (ilasc) :
review: Approve
Revision history for this message
Kristian Glass (doismellburning) wrote :

Much neater, thanks Colin

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/utilities/launchpad-database-setup b/utilities/launchpad-database-setup
2index 0b893de..46a36ce 100755
3--- a/utilities/launchpad-database-setup
4+++ b/utilities/launchpad-database-setup
5@@ -3,6 +3,8 @@
6 # Copyright 2009-2012 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9+set -e # Exit immediately if a command exits with a non-zero status.
10+
11 if [ -n "$1" ]; then
12 USER=$1
13 echo "Creating Launchpad database for $USER"
14@@ -21,8 +23,7 @@ fi
15 pgversion=
16 for try_pgversion in 10
17 do
18- sudo grep -qs "^auto" /etc/postgresql/$try_pgversion/main/start.conf
19- if [ $? -eq 0 ]; then
20+ if sudo grep -qs "^auto" /etc/postgresql/$try_pgversion/main/start.conf; then
21 pgversion="$try_pgversion"
22 break
23 fi
24@@ -37,8 +38,7 @@ fi
25 echo "Using postgres $pgversion"
26
27 # Make sure that we have the correct version running on port 5432
28-sudo grep -q "port.*5432" /etc/postgresql/$pgversion/main/postgresql.conf
29-if [ $? -ne 0 ]; then
30+if ! sudo grep -q "port.*5432" /etc/postgresql/$pgversion/main/postgresql.conf; then
31 echo "Please check /etc/postgresql/$pgversion/main/postgresql.conf and"
32 echo "ensure postgres is running on port 5432."
33 fi;

Subscribers

People subscribed via source and target branches

to status/vote changes: