[Maas-devel] [Merge] lp:~jtv/maas/single-cluster into lp:maas

[Gavin Panella]

Review: Needs Fixing

[1]

-build: bin/buildout
+build: bin/buildout dev-db

I know it's early days, but we probably don't want to create the dev
database for just a build.


[2]

-run:
+run: build
 	bin/django runserver 8000

If you agree with [1] then this needs to depend on build and dev-db.


[3]

-harness:
-	. bin/maasdb.sh ; maasdb_init_db db/development disposable
+harness: dev-db
 	bin/django shell
 
-syncdb:
+syncdb: dev-db
 	bin/django syncdb

However, these don't depend on build, so perhaps run doesn't need to
either.


[4]

+main() {
+    COMMAND="$1"

COMMAND will end up in the calling environment. The local command
sorts this out:

main() {
    local COMMAND="$1"

Please also let's not pretend we're writing for a plain /bin/sh; let's
just use bash and not go quite as insane. Or, read [6] and use Python,
or just about anything *but* shell.


[5]

+if test -n "$*"
+then
+    main $*
+fi

This won't work if sourced within a script that itself has args, or if
"set -- $something" has been used in the shell before sourcing this
script. Because main() defaults to doing nothing if the argument does
not match I think it's safe to just call main() unconditionally.


[6]

+    case $COMMAND in
+        start) maasdb_init_db $* ;;
+        stop) maasdb_stop_cluster $* ;;
+        query) maasdb_query $* ;;
+        shell) maasdb_shell $* ;;
+        delete-cluster) maasdb_delete_cluster $* ;;
+    esac
...
+    main $*

Avoid $* because it rarely does the right thing (in bash at
least). Also, more quotes needed. The code above behaves badly if
there are spaces in the arguments passed in. I think the following
will work without surprises:

main() {
    COMMAND="$1"
    shift
    case "$COMMAND" in
        start) maasdb_init_db "$@" ;;
        stop) maasdb_stop_cluster "$@" ;;
        query) maasdb_query "$@" ;;
        shell) maasdb_shell "$@" ;;
        delete-cluster) maasdb_delete_cluster "$@" ;;
    esac
}

if [ $# -gt 1 ]
then
    main "$@"
fi

Similar modifications ought to be made throughout the script.

To be honest, having to remember crap like this makes me avoid shell
script these days for anything other than the very most trivial
things.

-- 
https://code.launchpad.net/~jtv/maas/single-cluster/+merge/88739
Your team MaaS Developers is subscribed to branch lp:maas.