Skip to content

Commit

Permalink
RabbitMQ: remove validation of broker_parameters from profile (#4542)
Browse files Browse the repository at this point in the history
This validation was added as an attempt to help users with detecting
invalid parameters in the `broker_parameters` dictionary of a profile,
but `aiida-core` internally has no real limitations here. It is the
libraries underneath that decide what is acceptable and this can differ
from library to library plus it is not always clear. For example,
currently we use `topika` and `pika` which behave different from
`aiormq` which will be replacing them soon once `tornado` is replaced
with `asyncio`. It is best to not limit the options on `aiida-core`'s
side and just let it fail downstream as to not artificially limit the
parameters that might be perfectly acceptable by the libraries
downstream.
  • Loading branch information
sphuber authored Nov 5, 2020
1 parent a6c6cc4 commit 0a7039e
Show file tree
Hide file tree
Showing 9 changed files with 16 additions and 21 deletions.
19 changes: 3 additions & 16 deletions aiida/manage/external/rmq.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,6 @@
'heartbeat': 600,
})

BROKER_VALID_PARAMETERS = [
'heartbeat', # heartbeat timeout in seconds
'cafile', # string containing path to ca certificate file
'capath', # string containing path to ca certificates
'cadata', # base64 encoded ca certificate data
'keyfile', # string containing path to key file
'certfile', # string containing path to certificate file
'no_verify_ssl', # boolean disables certificates validation
]


def get_rmq_url(protocol=None, username=None, password=None, host=None, port=None, virtual_host=None, **kwargs):
"""Return the URL to connect to RabbitMQ.
Expand All @@ -66,14 +56,11 @@ def get_rmq_url(protocol=None, username=None, password=None, host=None, port=Non
:param host: the hostname of the RabbitMQ server.
:param port: the port of the RabbitMQ server.
:param virtual_host: the virtual host to connect to.
:param kwargs: remaining keyword arguments that will be encoded as query parameters.
:returns: the connection URL string.
"""
from urllib.parse import urlencode, urlunparse

invalid = set(kwargs.keys()).difference(BROKER_VALID_PARAMETERS)
if invalid:
raise ValueError('invalid URL parameters specified in the keyword arguments: {}'.format(', '.join(invalid)))

if 'heartbeat' not in kwargs:
kwargs['heartbeat'] = BROKER_DEFAULTS.heartbeat

Expand Down Expand Up @@ -186,14 +173,14 @@ def _continue(self, communicator, pid, nowait, tag=None):

try:
node = load_node(pk=pid)
except (exceptions.MultipleObjectsError, exceptions.NotExistent):
except (exceptions.MultipleObjectsError, exceptions.NotExistent) as exception:
# In this case, the process node corresponding to the process id, cannot be resolved uniquely or does not
# exist. The latter being the most common case, where someone deleted the node, before the process was
# properly terminated. Since the node is never coming back and so the process will never be able to continue
# we raise `Return` instead of `TaskRejected` because the latter would cause the task to be resent and start
# to ping-pong between RabbitMQ and the daemon workers.
LOGGER.exception('Cannot continue process<%d>', pid)
raise gen.Return(False)
raise gen.Return(False) from exception

if node.is_terminated:

Expand Down
7 changes: 7 additions & 0 deletions docs/source/intro/installation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,13 @@ The following parameters can be configured:
+--------------+---------------------------+---------------+-------------------------------------------------------------------------------------------------------------------------+
| Virtual host | ``--broker-virtual-host`` | ``''`` | Optional virtual host. Should not contain the leading forward slash, this will be added automatically by AiiDA. |
+--------------+---------------------------+---------------+-------------------------------------------------------------------------------------------------------------------------+
| Parameters | not available | n.a. | These are additional broker parameters that are typically encoded as URL parameters, for example, to specify SSL |
| | | | parameters such as the filepath to the certificate that is to be used. The parameters are currently not definable |
| | | | through the CLI but have to be added manually in the ``config.json``. A key ``broker_parameters`` should be added that |
| | | | is a dictionary, which can contain fields: ``cafile``, ``capath``, ``cadata``, ``certfile``, ``keyfile`` and |
| | | | ``no_verify_ssl``. |
+--------------+---------------------------+---------------+-------------------------------------------------------------------------------------------------------------------------+



verdi setup
Expand Down
1 change: 1 addition & 0 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ dependencies:
- sqlalchemy>=1.3.10,~=1.3
- tabulate~=0.8.5
- tornado<5.0
- topika~=0.2.2
- tqdm~=4.45
- tzlocal~=2.0
- upf_to_json~=0.9.2
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.5.txt
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ sympy==1.5.1
tabulate==0.8.6
terminado==0.8.3
testpath==0.4.4
topika==0.2.1
topika==0.2.2
tornado==4.5.3
tqdm==4.45.0
traitlets==4.3.3
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.6.txt
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ sympy==1.5.1
tabulate==0.8.6
terminado==0.8.3
testpath==0.4.4
topika==0.2.1
topika==0.2.2
tornado==4.5.3
tqdm==4.45.0
traitlets==4.3.3
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.7.txt
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ sympy==1.5.1
tabulate==0.8.6
terminado==0.8.3
testpath==0.4.4
topika==0.2.1
topika==0.2.2
tornado==4.5.3
tqdm==4.45.0
traitlets==4.3.3
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.8.txt
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ sympy==1.5.1
tabulate==0.8.6
terminado==0.8.3
testpath==0.4.4
topika==0.2.1
topika==0.2.2
tornado==4.5.3
tqdm==4.45.0
traitlets==4.3.3
Expand Down
1 change: 1 addition & 0 deletions setup.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"sqlalchemy~=1.3,>=1.3.10",
"tabulate~=0.8.5",
"tornado<5.0",
"topika~=0.2.2",
"tqdm~=4.45",
"tzlocal~=2.0",
"upf_to_json~=0.9.2",
Expand Down
1 change: 0 additions & 1 deletion tests/manage/external/test_rmq.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
@pytest.mark.parametrize(('args', 'kwargs', 'expected'), (
((), {}, 'amqp://guest:guest@127.0.0.1:5672?'),
((), {'heartbeat': 1}, 'amqp://guest:guest@127.0.0.1:5672?'),
((), {'invalid_parameters': 1}, ValueError),
((), {'cafile': 'file', 'cadata': 'ab'}, 'amqp://guest:guest@127.0.0.1:5672?'),
(('amqps', 'jojo', 'rabbit', '192.168.0.1', 6783), {}, 'amqps://jojo:rabbit@192.168.0.1:6783?'),
)) # yapf: disable
Expand Down

0 comments on commit 0a7039e

Please sign in to comment.