Skip to content

Commit

Permalink
Backport postgres improvements to deal with setups without sudo (#2433)
Browse files Browse the repository at this point in the history
Also updated `yapf` to recent version
  • Loading branch information
ltalirz authored and sphuber committed Feb 1, 2019
1 parent d8f8ff1 commit dde378e
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 80 deletions.
8 changes: 5 additions & 3 deletions .prospector.yaml
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
max-line-length: 120

ignore-paths:
- doc
- examples
- test
- utils

pylint:
max-line-length: 140

pyflakes:
run: false

pep8:
run: false

mccabe:
run: false
79 changes: 44 additions & 35 deletions aiida/backends/tests/verdi_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,23 +125,27 @@ def test_calculation_list(self):
calc_cmd.calculation_list()

out_str = ''.join(output)
self.assertTrue(calc_states.TOSUBMIT in out_str,
"The TOSUBMIT calculations should be part fo the "
"simple calculation list.")
self.assertTrue(calc_states.COMPUTED in out_str,
"The COMPUTED calculations should be part fo the "
"simple calculation list.")
self.assertFalse(calc_states.FINISHED in out_str,
"The FINISHED calculations should not be part fo the "
"simple calculation list.")
self.assertTrue(
calc_states.TOSUBMIT in out_str,
"The TOSUBMIT calculations should be part fo the "
"simple calculation list.")
self.assertTrue(
calc_states.COMPUTED in out_str,
"The COMPUTED calculations should be part fo the "
"simple calculation list.")
self.assertFalse(
calc_states.FINISHED in out_str,
"The FINISHED calculations should not be part fo the "
"simple calculation list.")

with Capturing() as output:
calc_cmd.calculation_list(*['-a'])

out_str = ''.join(output)
self.assertTrue(calc_states.FINISHED in out_str,
"The FINISHED calculations should be part fo the "
"simple calculation list.")
self.assertTrue(
calc_states.FINISHED in out_str,
"The FINISHED calculations should be part fo the "
"simple calculation list.")


class TestVerdiCodeCommands(AiidaTestCase):
Expand Down Expand Up @@ -193,12 +197,14 @@ def test_code_list(self):
with Capturing() as output:
code_cmd.code_list()
out_str_1 = ''.join(output)
self.assertTrue(computer_name_1 in out_str_1,
"The computer 1 name should be included into this list")
self.assertTrue(
computer_name_1 in out_str_1,
"The computer 1 name should be included into this list")
self.assertTrue(code_name_1 in out_str_1,
"The code 1 name should be included into this list")
self.assertTrue(computer_name_2 in out_str_1,
"The computer 2 name should be included into this list")
self.assertTrue(
computer_name_2 in out_str_1,
"The computer 2 name should be included into this list")
self.assertTrue(code_name_2 in out_str_1,
"The code 2 name should be included into this list")

Expand All @@ -207,16 +213,18 @@ def test_code_list(self):
with Capturing() as output:
code_cmd.code_list(*['-a'])
out_str_2 = ''.join(output)
self.assertEqual(out_str_1, out_str_2,
"verdi code list & verdi code list -a should provide "
"the same output in this experiment.")
self.assertEqual(
out_str_1, out_str_2,
"verdi code list & verdi code list -a should provide "
"the same output in this experiment.")

# Run a verdi code list -c, capture the output and check the result
with Capturing() as output:
code_cmd.code_list(*['-c', computer_name_1])
out_str = ''.join(output)
self.assertTrue(computer_name_1 in out_str,
"The computer 1 name should be included into this list")
self.assertTrue(
computer_name_1 in out_str,
"The computer 1 name should be included into this list")
self.assertFalse(
computer_name_2 in out_str,
"The computer 2 name should not be included into this list")
Expand All @@ -228,8 +236,9 @@ def test_code_list(self):
with Capturing() as output:
code_cmd.code_list()
out_str_3 = ''.join(output)
self.assertTrue(computer_name_1 in out_str_3,
"The computer 1 name should be included into this list")
self.assertTrue(
computer_name_1 in out_str_3,
"The computer 1 name should be included into this list")
self.assertTrue(code_name_1 in out_str_3,
"The code 1 name should be included into this list")
self.assertFalse(
Expand Down Expand Up @@ -665,12 +674,12 @@ def test_trajectory_simple_listing(self):

for nid in self.cmd_to_nodeid_map[sub_cmd]:
if str(nid) not in out_str:
self.fail("The data objects ({}) with ids {} and {} "
"were not found. "
.format(sub_cmd,
str(self.cmd_to_nodeid_map[sub_cmd][0]),
str(self.cmd_to_nodeid_map[sub_cmd][1])) +
"The output was {}".format(out_str))
self.fail(
"The data objects ({}) with ids {} and {} "
"were not found. ".format(
sub_cmd, str(self.cmd_to_nodeid_map[sub_cmd][0]),
str(self.cmd_to_nodeid_map[sub_cmd][1])) +
"The output was {}".format(out_str))

def test_trajectory_all_user_listing(self):
from aiida.cmdline.commands.data import _Bands
Expand Down Expand Up @@ -742,10 +751,10 @@ def test_trajectory_group_listing(self):
from aiida.cmdline.commands.data import _Cif
from aiida.cmdline.commands.data import _Trajectory

args_to_test = [['-g', self.group_name], [
'--group-name', self.group_name
], ['-G', str(self.group_id)], ['--group-pk',
str(self.group_id)]]
args_to_test = [['-g', self.group_name],
['--group-name', self.group_name],
['-G', str(self.group_id)],
['--group-pk', str(self.group_id)]]

sub_cmds = [_Bands, _Structure, _Cif, _Trajectory]
for sub_cmd in sub_cmds:
Expand All @@ -755,8 +764,8 @@ def test_trajectory_group_listing(self):
curr_scmd.list(*arg)
out_str = ' '.join(output)

if str(self.cmd_to_nodeid_map_for_groups[
sub_cmd]) not in out_str:
if str(self.
cmd_to_nodeid_map_for_groups[sub_cmd]) not in out_str:
self.fail(
"The data object ({}) with id {} "
"was not found. ".format(
Expand Down
71 changes: 52 additions & 19 deletions aiida/control/postgres.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
installed by default on various systems. If the postgres setup is not the
default installation, additional information needs to be provided.
"""

try:
import subprocess32 as subprocess
except ImportError:
Expand Down Expand Up @@ -64,17 +65,24 @@ class Postgres(object):
print('setup sucessful!')
"""

def __init__(self, port=None, interactive=False, quiet=True):
def __init__(self,
host='localhost',
port=None,
interactive=False,
quiet=True):
self.interactive = interactive
self.quiet = quiet
self.pg_execute = _pg_execute_not_connected
self.dbinfo = {}
if port:
self.set_port(port)
self.setup_fail_callback = None
self.setup_fail_counter = 0
self.setup_max_tries = 1

if host:
self.set_host(host)
if port:
self.set_port(port)

def set_setup_fail_callback(self, callback):
"""
Set a callback to be called when setup cannot be determined automatically
Expand All @@ -83,6 +91,10 @@ def set_setup_fail_callback(self, callback):
"""
self.setup_fail_callback = callback

def set_host(self, host):
"""Set the host manually"""
self.dbinfo['host'] = host

def set_port(self, port):
"""Set the port manually"""
self.dbinfo['port'] = str(port)
Expand All @@ -106,13 +118,19 @@ def determine_setup(self):
self.dbinfo = local_dbinfo
break

# This will work for the default Debian postgres setup
# This will work for the default Debian postgres setup, assuming that sudo is available to the user
if self.pg_execute == _pg_execute_not_connected:
dbinfo['user'] = 'postgres'
if _try_subcmd(
non_interactive=bool(not self.interactive), **dbinfo):
self.pg_execute = _pg_execute_sh
self.dbinfo = dbinfo
# Check if the user can find the sudo command
if _sudo_exists():
dbinfo['user'] = 'postgres'
if _try_subcmd(
non_interactive=bool(not self.interactive), **dbinfo):
self.pg_execute = _pg_execute_sh
self.dbinfo = dbinfo
else:
click.echo(
'Warning: Could not find `sudo`. No way of connecting to the database could be found.'
)

# This is to allow for any other setup
if self.pg_execute == _pg_execute_not_connected:
Expand Down Expand Up @@ -192,9 +210,9 @@ def db_exists(self, dbname):
def _no_setup_detected(self):
"""Print a warning message and calls the failed setup callback"""
message = '\n'.join([
'Detected no known postgres setup, some information is needed to create the aiida database and grant aiida access to it.',
'If you feel unsure about the following parameters, first check if postgresql is installed.',
'If postgresql is not installed please exit and install it, then run verdi quicksetup again.',
'Detected no known postgres setup, some information is needed to create the aiida database and grant ',
'aiida access to it. If you feel unsure about the following parameters, first check if postgresql is ',
'installed. If postgresql is not installed please exit and install it, then run verdi quicksetup again.',
'If postgresql is installed, please ask your system manager to provide you with the following parameters:'
])
if not self.quiet:
Expand Down Expand Up @@ -269,6 +287,22 @@ def _try_connect(**kwargs):
return success


def _sudo_exists():
"""
Check that the sudo command can be found
:return: True if successful, False otherwise
"""
try:
subprocess.check_output(['sudo', '-V'])
except subprocess.CalledProcessError:
return False
except OSError:
return False

return True


def _try_subcmd(**kwargs):
"""
try to run psql in a subprocess.
Expand All @@ -286,12 +320,12 @@ def _try_subcmd(**kwargs):


def _pg_execute_psyco(command, **kwargs):
'''
"""
executes a postgres commandline through psycopg2
:param command: A psql command line as a str
:param kwargs: will be forwarded to psycopg2.connect
'''
"""
from psycopg2 import connect, ProgrammingError
conn = connect(**kwargs)
conn.autocommit = True
Expand All @@ -316,7 +350,7 @@ def _pg_execute_sh(command, user='postgres', **kwargs):
:param kwargs: connection details to forward to psql, signature as in psycopg2.connect
To stop `sudo` from asking for a password and fail if one is required,
pass `noninteractive=True` as a kwarg.
pass `non_interactive=True` as a kwarg.
"""
options = ''
database = kwargs.pop('database', None)
Expand All @@ -330,7 +364,7 @@ def _pg_execute_sh(command, user='postgres', **kwargs):
if port:
options += '-p {}'.format(port)

## build command line
# Build command line
sudo_cmd = ['sudo']
non_interactive = kwargs.pop('non_interactive', None)
if non_interactive:
Expand All @@ -343,10 +377,9 @@ def _pg_execute_sh(command, user='postgres', **kwargs):
]
sudo_su_psql = sudo_cmd + su_cmd + psql_cmd
result = subprocess.check_output(sudo_su_psql, **kwargs)
result = result.decode('utf-8').strip().split('\n')
result = [i for i in result if i]

if isinstance(result, str):
result = result.strip().split('\n')
result = [i for i in result if i]
return result


Expand Down
8 changes: 4 additions & 4 deletions aiida/orm/data/cif.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,8 @@ class CifData(SinglefileData):
when setting ``ase`` or ``values``, a physical CIF file is generated
first, the values are updated from the physical CIF file.
"""
_set_incompatibilities = [('ase', 'file'), ('ase', 'values'), ('file',
'values')]
_set_incompatibilities = [('ase', 'file'), ('ase', 'values'),
('file', 'values')]
_scan_types = ['standard', 'flex']
_parse_policies = ['eager', 'lazy']

Expand Down Expand Up @@ -453,8 +453,8 @@ def get_or_create(cls, filename, use_first=False, store_cif=True):
else:
raise ValueError("More than one copy of a CIF file "
"with the same MD5 has been found in "
"the DB. pks={}".format(
",".join([str(i.pk) for i in cifs])))
"the DB. pks={}".format(",".join(
[str(i.pk) for i in cifs])))
else:
return cifs[0], False

Expand Down
12 changes: 3 additions & 9 deletions aiida/restapi/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ def __init__(self, **kwargs):
# Configure utils
utils_conf_keys = ('PREFIX', 'PERPAGE_DEFAULT', 'LIMIT_DEFAULT')
self.utils_confs = {
k: kwargs[k]
for k in utils_conf_keys
if k in kwargs
k: kwargs[k] for k in utils_conf_keys if k in kwargs
}
self.utils = Utils(**self.utils_confs)

Expand Down Expand Up @@ -103,9 +101,7 @@ def __init__(self, **kwargs):
# Configure utils
utils_conf_keys = ('PREFIX', 'PERPAGE_DEFAULT', 'LIMIT_DEFAULT')
self.utils_confs = {
k: kwargs[k]
for k in utils_conf_keys
if k in kwargs
k: kwargs[k] for k in utils_conf_keys if k in kwargs
}
self.utils = Utils(**self.utils_confs)
self.method_decorators = {'get': kwargs.get('get_decorators', [])}
Expand Down Expand Up @@ -205,9 +201,7 @@ def __init__(self, **kwargs):
# Configure utils
utils_conf_keys = ('PREFIX', 'PERPAGE_DEFAULT', 'LIMIT_DEFAULT')
self.utils_confs = {
k: kwargs[k]
for k in utils_conf_keys
if k in kwargs
k: kwargs[k] for k in utils_conf_keys if k in kwargs
}
self.utils = Utils(**self.utils_confs)
self.method_decorators = {'get': kwargs.get('get_decorators', [])}
Expand Down
18 changes: 9 additions & 9 deletions docs/update_req_for_rtd.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ def update_req_for_rtd(pre_commit):

import setup_requirements

reqs = set(
setup_requirements.extras_require['docs'] + setup_requirements.
extras_require['rest'] + setup_requirements.extras_require['testing'] +
setup_requirements.extras_require[':python_version < "3"'] +
# To avoid that it requires also the postgres libraries
[
p for p in setup_requirements.install_requires
if not p.startswith('psycopg2')
])
reqs = set(setup_requirements.extras_require['docs'] +
setup_requirements.extras_require['rest'] +
setup_requirements.extras_require['testing'] +
setup_requirements.extras_require[':python_version < "3"'] +
# To avoid that it requires also the postgres libraries
[
p for p in setup_requirements.install_requires
if not p.startswith('psycopg2')
])
reqs_str = "\n".join(sorted(reqs))

basename = 'requirements_for_rtd.txt'
Expand Down
Loading

0 comments on commit dde378e

Please sign in to comment.