bugfix: when set config result_expires = 0, chord.get will hang.

[laixintao]

EXPIRE key 0 will delete a key in redis, then chord will never get the
result.

fix: #5237

Description

From the doc:

A value of None or 0 means results will never expire (depending on backend specifications).

https://docs.celeryproject.org/en/stable/userguide/configuration.html#std-setting-result_expires

But here only checked with None, 0 was missed.

[lgtm-com]

This pull request fixes 2 alerts when merging 8d0600e into ea37db1 - view on LGTM.com

fixed alerts:

• 1 for Non-exception in 'except' clause
• 1 for Module is imported with 'import' and 'import from'

[auvipy]

can you also add a unit test for this change?

[laixintao]

sure

[laixintao]

I just asset that expire shouldn't call when config with zero.

[laixintao]

So I wrote a testcase, without this PR, it failed:

➜ t2 celery git:(bugfix/chord-expire) ✗ pytest t/unit/backends/test_redis.py
================================ test session starts ================================
platform darwin -- Python 3.8.2, pytest-6.1.0, py-1.9.0, pluggy-0.13.1
rootdir: /Users/laixintao/Program/celery, configfile: setup.cfg
plugins: celery-0.0.0a1, case-1.5.3, timeout-1.4.2
collected 65 items

t/unit/backends/test_redis.py ..........................................F.... [ 72%]
..................                                                            [100%]

===================================== FAILURES ======================================
__________ test_RedisBackend.test_on_chord_part_return_expire_set_to_zero ___________

self = <t.unit.backends.test_redis.test_RedisBackend object at 0x10bc9b7f0>
restore = <MagicMock name='restore' id='4493273168'>

    @patch('celery.result.GroupResult.restore')
    def test_on_chord_part_return_expire_set_to_zero(self, restore):
        old_expires = self.b.expires
        self.b.expires = 0
        tasks = [self.create_task(i) for i in range(10)]

        for i in range(10):
            self.b.on_chord_part_return(tasks[i].request, states.SUCCESS, i)
            assert self.b.client.zadd.call_count
            self.b.client.zadd.reset_mock()
        assert self.b.client.zrangebyscore.call_count
        jkey = self.b.get_key_for_group('group_id', '.j')
        tkey = self.b.get_key_for_group('group_id', '.t')
        self.b.client.delete.assert_has_calls([call(jkey), call(tkey)])
>       self.b.client.expire.assert_not_called()

t/unit/backends/test_redis.py:729:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <Mock name='Redis.expire' id='4493402320'>

    def assert_not_called(self):
        """assert that the mock was never called.
        """
        if self.call_count != 0:
            msg = ("Expected '%s' to not have been called. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
>           raise AssertionError(msg)
E           AssertionError: Expected 'expire' to not have been called. Called 20 times.
E           Calls: [call(b'celery-taskset-meta-group_id.j', 0),
E            call(b'celery-taskset-meta-group_id.t', 0),
E            call(b'celery-taskset-meta-group_id.j', 0),
E            call(b'celery-taskset-meta-group_id.t', 0),
E            call(b'celery-taskset-meta-group_id.j', 0),
E            call(b'celery-taskset-meta-group_id.t', 0),
E            call(b'celery-taskset-meta-group_id.j', 0),
E            call(b'celery-taskset-meta-group_id.t', 0),
E            call(b'celery-taskset-meta-group_id.j', 0),
E            call(b'celery-taskset-meta-group_id.t', 0),
E            call(b'celery-taskset-meta-group_id.j', 0),
E            call(b'celery-taskset-meta-group_id.t', 0),
E            call(b'celery-taskset-meta-group_id.j', 0),
E            call(b'celery-taskset-meta-group_id.t', 0),
E            call(b'celery-taskset-meta-group_id.j', 0),
E            call(b'celery-taskset-meta-group_id.t', 0),
E            call(b'celery-taskset-meta-group_id.j', 0),
E            call(b'celery-taskset-meta-group_id.t', 0),
E            call(b'celery-taskset-meta-group_id.j', 0),
E            call(b'celery-taskset-meta-group_id.t', 0)].

/usr/local/var/pyenv/versions/3.8.2/lib/python3.8/unittest/mock.py:874: AssertionError
============================== short test summary info ==============================
FAILED t/unit/backends/test_redis.py::test_RedisBackend::test_on_chord_part_return_expire_set_to_zero
=========================== 1 failed, 64 passed in 1.26s ============================

With this PR, it passed! ✨ I think it's enough, what do you think? @auvipy

Test implemented.

[thedrow]

LGTM.

Thank you!