Skip to content

Commit

Permalink
[#2733] Allow paster command to work across different postgres instal…
Browse files Browse the repository at this point in the history
…lations.

The "postgres" user isn't always the superuser.
  • Loading branch information
icmurray committed Sep 14, 2012
1 parent 74684d2 commit ce424a8
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 7 deletions.
35 changes: 29 additions & 6 deletions ckanext/datastore/commands.py
Expand Up @@ -39,8 +39,15 @@ class SetupDatastoreCommand(CkanCommand):
Usage::
create-db - create the datastore database
create-read-only-user - create a read-only user for the datastore read url
paster datastore create-db <sql-user-user>
paster datastore create-read-only-user <sql-super-user>
Where:
<sql-super-user> is the name of a postgres user with sufficient
permissions to create new tables, users, and grant
and revoke new permissions. Typically, this would
be the "postgres" user.
'''
summary = __doc__.split('\n')[0]
usage = __doc__
Expand All @@ -67,15 +74,25 @@ def command(self):
assert self.urlparts_w['db_name'] == self.urlparts_r['db_name'], "write and read db should be the same"

if cmd == 'create-db':
if len(self.args) != 2:
print self.usage
return
self.sql_superuser = self.args[1]
self.create_db()
if self.verbose:
print 'Creating DB: SUCCESS'
elif cmd == 'create-read-only-user':
if len(self.args) != 2:
print self.usage
return
self.sql_superuser = self.args[1]
self.create_read_only_user()
if self.verbose:
print 'Creating read-only user: SUCCESS'
else:
print self.usage
log.error('Command "%s" not recognized' % (cmd,))
return

def _get_db_config(self, name):
from pylons import config
Expand All @@ -93,15 +110,19 @@ def _run_cmd(self, command_line):
if retcode != 0:
raise SystemError('Command exited with errorcode: %i' % retcode)

def _run_sql(self, sql, database='postgres'):
def _run_sql(self, sql, as_sql_user, database='postgres'):
if self.verbose:
print "Executing: \n#####\n", sql, "\n####\nOn database:", database
if not self.simulate:
self._run_cmd("sudo -u postgres psql {database} -c '{sql}'".format(sql=sql, database=database))
self._run_cmd("psql --username='{username}' --dbname='{database}' -c '{sql}' -W".format(
username=as_sql_user,
database=database,
sql=sql
))

def create_db(self):
sql = "create database {0}".format(self.urlparts_w['db_name'])
self._run_sql(sql)
self._run_sql(sql, as_sql_user=self.sql_superuser)

def create_read_only_user(self):
sql = read_only_user_sql.format(
Expand All @@ -110,5 +131,7 @@ def create_read_only_user(self):
ckanuser=self.urlparts_c['db_user'],
readonlyuser=self.urlparts_r['db_user'],
writeuser=self.urlparts_w['db_user'])
self._run_sql(sql, self.urlparts_w['db_name'])
self._run_sql(sql,
as_sql_user=self.sql_superuser,
database=self.urlparts_w['db_name'])

2 changes: 1 addition & 1 deletion ckanext/datastore/plugin.py
Expand Up @@ -37,7 +37,7 @@ def configure(self, config):
# Check whether we are running one of the paster commands which means
# that we should ignore the following tests.
import sys
if sys.argv[-1] in ['create-db', 'create-read-only-user']:
if sys.argv[0].split('/')[-1] == 'paster':

This comment has been minimized.

Copy link
@domoritz

domoritz Sep 17, 2012

Contributor

This is not exactly the same since tests are not only skipped if you run the paster create-db or create-read-only-user commands. It skips the tests even if you run ckan with paster serve --reload development.ini. I know the previous solution was a little bit dodgy but the easiest and safest in my opinion.

This comment has been minimized.

Copy link
@icmurray

icmurray Sep 17, 2012

Author Contributor

Sorry @domoritz, I agree. That's not really a sensible idea after all. How about checking if argv[1] == "datastore" as well? I was just trying to allow arguments to be passed to the datastore paster commands (ie - not checking the last argument), as it's quite common to be able to pass in the config file as an argument.

This comment has been minimized.

Copy link
@domoritz

domoritz Sep 17, 2012

Contributor

That is why I tried to avoid additional comments ;-) What about also checking whether create-db or create-read-only-user are in sys.argv? I think it's better to make the check as detailed as possible so that you don't have false negatives. Other than that, argv[1] == "datastore" makes sense to me.

I don't know that much about paster. Are there other optional commands like verbose output that could destroy our ideas?

This comment has been minimized.

Copy link
@icmurray

icmurray Sep 17, 2012

Author Contributor

Checking for the actual datastore commands is fine by me :-)

I agree that this is all a bit flakey though. One thought: there's a IConfigurer interface that could be used to alter the config when the datastore paster command runs. This would mean loading such a plugin prior to the _load_config() call (I think it's named that). It'd be cleaner than trying to parse sys.argv, but I don't know if it will work though: I have a feeling that plugins are cleared prior to loading all the ones from the config file. :-(

This comment has been minimized.

Copy link
@domoritz

domoritz Sep 17, 2012

Contributor

It's not such an important part so I think parsing argv is okayish as long as we don't have false negatives.

log.warn("Omitting permission checks because you are "
"running paster commands.")
return
Expand Down

0 comments on commit ce424a8

Please sign in to comment.