search

aiidateam / kiwipy

A python messaging library for RPC, task queues and broadcasts
https://kiwipy.readthedocs.io
Other
15 stars 4 forks source link

fix: lock down aio-pika dependency #110

Closed ltalirz closed 1 year ago

ltalirz commented 1 year ago

fixes #94 fixes #111 see also #112

CI tests fail with recent versions of aio-pika (e.g. version 8) due to API changes.

Furthermore, the Jupyter notebook test from the test suite seems to reproducibly fail with aio-pika version 6.8.2 but pass with aio-pika 6.8.1 (see commits in PR).

Also, kiwipy depends directly on pamqp because of this single check:

https://github.com/aiidateam/kiwipy/blob/adf373e794ed69d5ec21d4875514971f32d7734f/kiwipy/rmq/threadcomms.py#L261

This dependency was not made explicit in the setup.py, and the pre-commit pylint checks alert us to the fact that the API of pamqp 3 has changed.

ltalirz commented 1 year ago

Hm... @unkcpz looks like the jupyter notebook test now fails across all python/rabbitmq versions.

It's not clear to me what changed - the last passing commit is from Jan 17 2022. At that point, the latest aio-pika release was 6.8.1 (here we're now installing 6.8.2); I can't really imagine that this is the difference?

The only other thing I notice is that aio-pika 6 is using a somewhat dated version of aiormq (3.3.1) from 2020. Today, aiormq 6 is already out.

codecov[bot] commented 1 year ago

Codecov Report

Merging #110 (915fdda) into develop (adf373e) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #110   +/-   ##
========================================
  Coverage    90.32%   90.32%           
========================================
  Files           16       16           
  Lines         1146     1146           
========================================
  Hits          1035     1035           
  Misses         111      111

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ltalirz commented 1 year ago

Wow - either the problem was introduced in aio-pika 6.8.2 or the test is not reproducible.

ltalirz commented 1 year ago

@sphuber @chrisjsewell Could one of you review this please?

ltalirz commented 1 year ago

Just for the record, the jupyter notebook test also fails with the combination aio-pika 6.8.2 and pamqp 2 (which was not tested in this PR).

ltalirz commented 1 year ago

Correct

sphuber commented 1 year ago

Do you guys mean that this PR with upper limit <6.8.2 needs a patch release and then #113 with ~=8.0 should be a new minor release?

ltalirz commented 1 year ago

Yes (upper limit is both for aio-pika and for pamqp, both of which currently break kiwipy)

sphuber commented 1 year ago

Ok great, thanks. Want me to take care of merging and making a release?

ltalirz commented 1 year ago

That would be great, thanks Seb!

sphuber commented 1 year ago

v0.7.6 is now released. By the way, just noticed that this fixes #94 :+1: