From 0a7039e99bbdceba5148443d279e26ca9003b66a Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Thu, 5 Nov 2020 03:02:52 -0800 Subject: [PATCH] RabbitMQ: remove validation of `broker_parameters` from profile (#4542) 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. --- aiida/manage/external/rmq.py | 19 +++---------------- docs/source/intro/installation.rst | 7 +++++++ environment.yml | 1 + requirements/requirements-py-3.5.txt | 2 +- requirements/requirements-py-3.6.txt | 2 +- requirements/requirements-py-3.7.txt | 2 +- requirements/requirements-py-3.8.txt | 2 +- setup.json | 1 + tests/manage/external/test_rmq.py | 1 - 9 files changed, 16 insertions(+), 21 deletions(-) diff --git a/aiida/manage/external/rmq.py b/aiida/manage/external/rmq.py index 51ffe1272c..9746b72df8 100644 --- a/aiida/manage/external/rmq.py +++ b/aiida/manage/external/rmq.py @@ -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. @@ -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 @@ -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: diff --git a/docs/source/intro/installation.rst b/docs/source/intro/installation.rst index 48e7afccbe..e3032bb0b2 100644 --- a/docs/source/intro/installation.rst +++ b/docs/source/intro/installation.rst @@ -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 diff --git a/environment.yml b/environment.yml index bcd6fa463e..07f2d6aba0 100644 --- a/environment.yml +++ b/environment.yml @@ -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 diff --git a/requirements/requirements-py-3.5.txt b/requirements/requirements-py-3.5.txt index ecf765f40d..e25b658168 100644 --- a/requirements/requirements-py-3.5.txt +++ b/requirements/requirements-py-3.5.txt @@ -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 diff --git a/requirements/requirements-py-3.6.txt b/requirements/requirements-py-3.6.txt index c8a0a17a72..7bfdeb93c4 100644 --- a/requirements/requirements-py-3.6.txt +++ b/requirements/requirements-py-3.6.txt @@ -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 diff --git a/requirements/requirements-py-3.7.txt b/requirements/requirements-py-3.7.txt index 6dbcea29d3..ef4f89ad4d 100644 --- a/requirements/requirements-py-3.7.txt +++ b/requirements/requirements-py-3.7.txt @@ -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 diff --git a/requirements/requirements-py-3.8.txt b/requirements/requirements-py-3.8.txt index 01259cf649..3c878cbf7a 100644 --- a/requirements/requirements-py-3.8.txt +++ b/requirements/requirements-py-3.8.txt @@ -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 diff --git a/setup.json b/setup.json index 3de06edf07..d5927c5604 100644 --- a/setup.json +++ b/setup.json @@ -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", diff --git a/tests/manage/external/test_rmq.py b/tests/manage/external/test_rmq.py index 5c27e6962f..22294499ad 100644 --- a/tests/manage/external/test_rmq.py +++ b/tests/manage/external/test_rmq.py @@ -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