aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
414 stars 187 forks source link

CI: improve testing for various supported RabbitMQ configurations #4591

Closed sphuber closed 2 months ago

sphuber commented 3 years ago

The CI already contains a workflow rabbitmq.yml that targets the testing of various RabbitMQ configurations, but currently the only variation is the version of the RabbitMQ server that is being tested against. Ideally, we should also test that connections over SSL works, that different virtual hosts and non-default user credentials function properly.

ltalirz commented 3 years ago

@sphuber @chrisjsewell Just wondering: is what is being tested here really specific to aiida-core or would this better be tested in kiwipy or some other dependency?

With this new workflow, every change to AiiDA triggers these rabbitmq-compatibility tests, while the fraction of changes at the aiida-core level that actually have the chance of affecting this is probably close to zero (?).

In the case of postgres, I factored out the pgsu package for a similar purpose (test the commands we need for a wide range of postgresql installations)

chrisjsewell commented 3 years ago

Indeed, looking at https://github.com/aiidateam/aiida-core/blob/develop/tests/manage/external/test_rmq.py, it does feel this should be moved to kiwipy

sphuber commented 3 years ago

I agree that in its current state, the tests could be moved to kiwipy but that is exactly why I opened this issue. The need for these tests came up when adding SSL support for LLNL, which not only showed a few bugs in the various layers we have (aiida-core -> kiwipy -> topika -> pika) but also that we need automated tests to check that things still work even if one of the intermediate layers change. For example, with the migration to asyncio, we no longer use tornado and so no longer use topika nor pika. Instead we use aio-pika through aiormq (I know, the number of layers is nuts). I am not at all certain that what I did to make SSL work with topika and pika still works now. Part of this can be tested in kiwipy but I think we would still need this in aiida-core as well, as it is here that we build the RabbitMQ URL based on profile settings. Given that RabbitMQ working properly is crucial for AiiDA working, I figured this merits automated tests.

chrisjsewell commented 3 years ago

fair

ltalirz commented 3 years ago

If I understand correctly, then this would be solved if kiwipy had a function to construct the RabbitMQ URL that is used from aiida-core and was properly tested for all inputs (and rabbitmq versions) as part of the kiwipy CI (?).

sphuber commented 3 years ago

If I understand correctly, then this would be solved if kiwipy had a function to construct the RabbitMQ URL that is used from aiida-core and was properly tested for all inputs (and rabbitmq versions) as part of the kiwipy CI (?).

In a sense, yes, although we would still need some tests in aiida-core but they could be a lot lighter than we have now and they don't need to be tested against different versions of RabbitMQ.

Essentially, kiwipy allows you to specify the connection to RabbitMQ through a URL string or through keyword arguments. In aiida-core we currently just format the string and pass this. This is done in aiida.manage.external.rmq.get_rmq_url. We should still test that for all the potential profile configurations (all the broker_* settings) we format the "correct" url. This should be a light test though and can become part of the normal unit tests and workflow again. We should then make sure in kiwipy that these strings are properly propagated to RabbitMQ through the lower lying libraries and make sure that they work for various versions of RabbitMQ and various setups (with/without SSL, different virtual hosts, different user credentials etc.)