Skip to content

Commit

Permalink
Set the STAGING_SITE config based on install type
Browse files Browse the repository at this point in the history 
DEVELOPMENT_INSTALL is exported by site-specific-install. Choose the
correct STAGING_SITE setting based on this variable.
  • Loading branch information
@garethrees
garethrees committed May 28, 2015
1 parent b7a40c1 commit 9c66a11
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions script/install-as-user
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ RANDOM_EMAIL_SECRET=$(random_alphanumerics 32)
RANDOM_EMERGENCY_PASSWORD=$(random_alphanumerics 10)
RANDOM_COOKIE_SECRET=$(random_alphanumerics 100)

if [ $DEVELOPMENT_INSTALL = true ]; then

This comment has been minimized.

Sign in to view

Copy link
@crowbot

crowbot May 29, 2015

Member

There are a few syntax issues here. $DEVELOPMENT_INSTALL isn't passed to this script, so isn't defined at this point. Also, this logic seems inverted - if we got the DEVELOPMENT_INSTALL flag, then STAGING_SITE should be 1, not 0.

This comment has been minimized.

Sign in to view

Copy link
@garethrees

garethrees May 29, 2015

Author Member

Its exported just before running install-as-user https://github.com/mysociety/alaveteli/blob/rails-3-develop/script/site-specific-install.sh#L192-L193. If its not defined it uses the else.

Oops, quite right about the ordering.

This comment has been minimized.

Sign in to view

Copy link
@garethrees

garethrees May 29, 2015

Author Member

also need to quote $DEVELOPMENT_INSTALL

This comment has been minimized.

Sign in to view

Copy link
@crowbot

crowbot May 29, 2015

Member

Ah, I hadn't seen the significance of the export. However, when $DEVELOPMENT_INSTALL is undefined, the script produces the warning "[: =: unary operator expected", as the syntax effectively expands to

if [ = true ]

so you should quote the $DEVELOPMENT_INSTALL variable:

 if [ "$DEVELOPMENT_INSTALL" = true ]; then
STAGING_SITE="0"
else
STAGING_SITE="1"
fi

if ! [ -f config/general.yml ]
then
sed -r \
Expand Down Expand Up @@ -115,6 +121,7 @@ then
-e "s,^( *THEME_BRANCH:).*,\\1 'develop'," \
-e "s,^( *USE_MAILCATCHER_IN_DEVELOPMENT:).*,\\1 false," \
-e "s,^( *BUNDLE_PATH:).*,\\1 $HOME/bundle/," \
-e "s,^( *STAGING_SITE:).*,\\1 '$STAGING_SITE'," \
config/general.yml-example > config/general.yml
fi

Expand Down

2 comments on commit 9c66a11

@crowbot
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems reasonable (once the logic is fixed), but I think it needs a couple of other corresponding changes to keep things working as expected, and as documented. The installation script documentation says that this will install a site in development mode by default - I think we should update the instructions on that page so that they show the --development flag being used, and then give instructions for what to do if you want to install in production mode. Our own instructions for updating the AMI also now need to be updated to pass the --development flag, as the documentation for the AMI also says it runs in development mode by default.

@garethrees
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, but wow that's confusing! Maybe this needs a bit more thought to ensure the script is suitable for a non-development install.

Please sign in to comment.