[bug] missing 'memcache' dependency in 1.0.0b2

[woutdenolf]

This works

pip install "pytest-celery==1.0.0b1" celery
pytest -h

This fails

pip install "pytest-celery==1.0.0b2" celery
pytest -h

  File "/home/denolf/virtualenvs/pybox_9uxen5/lib/python3.8/site-packages/_pytest/assertion/rewrite.py", line 175, in exec_module
    exec(co, module.__dict__)
  File "/home/denolf/virtualenvs/pybox_9uxen5/lib/python3.8/site-packages/pytest_celery/__init__.py", line 28, in <module>
    from pytest_celery.vendors.memcached.container import *
  File "/home/denolf/virtualenvs/pybox_9uxen5/lib/python3.8/site-packages/_pytest/assertion/rewrite.py", line 175, in exec_module
    exec(co, module.__dict__)
  File "/home/denolf/virtualenvs/pybox_9uxen5/lib/python3.8/site-packages/pytest_celery/vendors/memcached/container.py", line 9, in <module>
    import memcache
ModuleNotFoundError: No module named 'memcache'

celery 5.3.6 and pytest 8.0.0 in both cases.

[Nusnus]

Try: pip install "pytest-celery[celery]==1.0.0b2”

[woutdenolf]

Aha, thanks @Nusnus !

I have a package that depends on celery and has pytest-celery in its test dependencies. It is somewhat confusing to need pytest-celery[celery] isn't it? What changed in 1.0.0b1 that this is needed?

[woutdenolf]

Ok the celery dependency was made optional in b5e23ad.

It is still a bit unusual compared to other pytest plugins like pytest-redis (which depends in redis) or pytest-mongo (which depends on pymongo).

What use case does pytest-celery have without celery?

[woutdenolf]

#172 (comment)

For testing and simple use, pip install "pytest-celery[celery]" to take the latest Celery package version.
For production environments, celery should already be installed by itself with the versioning and packages the environment uses, so pip install pytest-celery should be enough to avoid installing useless packages.

So this means pip install pytest-redis in production? But then you get No module named 'memcache'.

[Nusnus]

Hey @woutdenolf , this is indeed a subject I am very happy to discuss about, so thank you for bringing up this subject.
Also, I’ve seen the PR - let’s discuss this issue first before moving to code changes, I’d like to hear your feedback on my design choice first.

So, originally, I was under the same perception you suggested, where pytest-celery is “invalid” without the celery package as well, so I added it as a dependency for the latest Celery version. This was done for the Beta 1 release, as mentioned before.

#172 (comment)

For testing and simple use, pip install "pytest-celery[celery]" to take the latest Celery package version.
For production environments, celery should already be installed by itself with the versioning and packages the environment uses, so pip install pytest-celery should be enough to avoid installing useless packages.

So this means pip install pytest-redis in production? But then you get No module named 'memcache'.

Let me expand on my PR description/comment.

For production environments, celery should already be installed by itself with the versioning and packages the environment uses, so pip install pytest-celery should be enough to avoid installing useless packages.

A production environment may have infinite combinations of tech stacks, which may include official and/or modified Celery workers clusters. Such production environments have their own specific dependencies and complexities. So, to avoid even attempting to take responsibility for that, the design choice was to install the pytest-celery plugin as a layer on top of your currently existing infrastructure, which already installs Celery with its required dependencies, etc. by definition.

For testing and simple use, pip install "pytest-celery[celery]" to take the latest Celery package version.

Another very common use-case would be to “just get my EVERYTHING ready one-click no stop” to POC ideas and test more simple Celery use-cases, with out-of-the-box setup. For that, the celery extra was added to allow easy installation of a full environment, ready to File -> New -> test_POC.py, with all of the required dependencies and configurations built-in.

So this means pip install pytest-redis in production? But then you get No module named 'memcache’.

Great point. The design choice for using the celery extra, which uses itself the redis and pymemcache (which is actually python-memcached and not pymemcache !!) Celery extras, is to make sure the tested environment will match the same package versions the testing environment has installed, for the default pytest-celery[celery] installation. For this reason, I prefer to use the extras from celery[redis,pymemcache,…] implicitly and not explicitly install it as a dependency.

What use case does pytest-celery have without celery?

Based on the explanation above then, if you use the pytest-celery in a production environment that doesn’t use 100% all defaults on everything, then you’re expected to have an environment that already has your dependencies ready (Redis, Memcached, amqp, etc.) and installing pytest-celery on that environment (without the Celery extra) will just give you the testing features from the plugin, but will make sure they are also using your environment’s package installation.

For example, the new smoke tests are installing pytest-celery without the celery extra because we want to use the current source code for the Celery installation.

pytest-celery==1.0.0b2

The smoke tests environment defines a smoke tests worker that does NOT use the built-in default latest Celery version worker from the plugin but instead uses the “production-style” of taking responsibility for the worker installation by itself to make sure the installed Celery version is done using edit mode on the current source code.

RUN pip install --no-cache-dir --upgrade \
    pip \
    -e /celery[redis,pymemcache] \
    psutil

Where the source code is loaded via a volume:

volumes={
        # Volume: Worker /app
        "{default_worker_volume.name}": defaults.DEFAULT_WORKER_VOLUME,
        # Mount: Celery source
        os.path.abspath(os.getcwd()): {
            "bind": "/celery",
            "mode": "rw",
        },
    },

So to reiterate,

So this means pip install pytest-redis in production? But then you get No module named 'memcache’.

This means then that you’re responsible for installing the packages your environment uses so as to match testing/tested package installation.

If you’re installing without the celery extra, DO install the dependencies you use (e.g redis), don’t test for the memcached backend configuration (disabled by default right now), and still get the missing module error then there’s a real bug, which correctly matches this issue’s subject so I will also reopen this issue until we can come to understanding or new conclusions.

I hope I made it all clear - feel free to ask anything. I welcome any productive feedback!

[Nusnus]

@woutdenolf, does my reasoning make sense to you?

[woutdenolf]

If you’re installing without the celery extra, DO install the dependencies you use (e.g redis), don’t test for the memcached backend configuration (disabled by default right now), and still get the missing module error then there’s a real bug,

That is exactly the problem. Or vice versa, a project has celery[pymemcache] as dependency, does not need Redis testing but still gets redis import errors. So we agree: pytest-celery still needs to work when neither redis nor python-memcached are installed. This only question left: should celery itself be an optional dependency?

I prefer to use the extras from celery[redis,pymemcache,…] implicitly and not explicitly install it as a dependency.

Agreed. I just couldn't find a way to do it with poetry but them found python-poetry/poetry-core#613 (comment).

... taking responsibility for the worker installation by itself to make sure the installed Celery version is done using edit mode on the current source code.

Still I don't understand why having celery as a required dependency would change any custom way of installing celery before installing pytest-test. As I understand, the smoke tests use an editable install of celery.

pip install -e /celery[redis,pymemcache] ...

pip install celery  # this will do nothing because celery is already installed

Correct me if I'm wrong, but I see no problem here.

Let me put it differently, is there a way in which we can make celery importable

from celery import Celery

and then pip install celery would mess something up?

I guess the only way this could be done is making celery importable without pip installing it. For example by using the PYTHONPATH

export PYTHONPATH=/home/user/projects/celery

cd /home/user
pip install celery  

Requirement already satisfied: celery in ./projects/celery (5.3.0rc1)  <<<<  we do not install celery
Collecting billiard<5.0,>=4.1.0 (from celery)
  Using cached billiard-4.2.0-py3-none-any.whl.metadata (4.4 kB)
Collecting kombu<6.0,>=5.3.0b3 (from celery)
  Using cached kombu-5.3.5-py3-none-any.whl.metadata (3.1 kB)
...
Installing collected packages: wcwidth, vine, tzdata, six, prompt-toolkit, click, billiard, python-dateutil, click-repl, click-plugins, click-didyoumean, amqp, kombu
Successfully installed amqp-5.2.0 billiard-4.2.0 click-8.1.7 click-didyoumean-0.3.0 click-plugins-1.1.1 click-repl-0.3.0 kombu-5.3.5 prompt-toolkit-3.0.43 python-dateutil-2.8.2 six-1.16.0 tzdata-2024.1 vine-5.1.0 wcwidth-0.2.13
<<<< we do install missing celery dependencies as specific in the requirements of celery on pypi

So even this won't install celery. It does install missing celery core dependencies. But I don't consider this a legitimate use case.

Then there is the upper limit on the celery version

[tool.poetry.dependencies]
celery = { version = "<6.0.0" }

Could this cause problems? Suppose we have this

pip install -e ./celery  # assume the celery version is 6.0

pip install "celery<6.0.0"  # this will indeed install celery from pypi

But if pytest-celery cannot be used for celery>=6.0.0, how are we in the situation of using it to test celery 6.0? In other words, the problem is not solved by making celery an optional dependency of pytest-celery, we shifted the problem somewhere else.

[Nusnus]

@woutdenolf

and then pip install celery would mess something up?

My assumption is: Yes.
Example: Installing "my celery clone" without some default dependencies my target env can’t handle, then installing official celery will cause these deps to be installed anyways implicitly which is wrong.

This is why wanted the pytest-celery plugin (without extras) to be celery-agnostic, so you’ll have explicit control over the celery[extras] version you’re using. To add on that, I didn’t want to explicitly add the celery extras themselves as pytest-celery dependencies, assuming that if you use a component/API that uses a package you don’t have installed, you will get an error, indicating you should fix your environment. This is useful for avoiding the implicit use of components you didn’t plan for (due to automatic/default configurations), or a reminder to add the missing packages if you do want all of the automatic components by default (which causes an expected dependencies gap which means the error is fine).

The fix should be configuring the plugin to your setup by code (conftest.py) and installing the required dependencies according to that setup (e.g., Redis + Memcached, RabbitMQ only, etc.). For example, configure your test environment to use only Redis broker/backend, then install only redis package, then everything should work and the RabbitMQ API for example shouldn’t be called which is fine and would avoid missing imports error, because you don’t use nor install the RabbitMQ stuff.

But if pytest-celery cannot be used for celery>=6.0.0, how are we in the situation of using it to test celery 6.0?
🎯🎯🎯

Needs to be removed indeed. Good point!

Now regarding,

• make celery a required dependency and the celery extra's optional dependencies #206

and

That is exactly the problem. Or vice versa, a project has celery[pymemcache] as dependency, does not need Redis testing but still gets redis import errors. So we agree: pytest-celery still needs to work when neither redis nor python-memcached are installed. This only question left: should celery itself be an optional dependency?

Ok, so let’s see how we deal with this forward.

1. I still argue that the pytest-celery should NOT INSTALL celery by default due to packages sensitivity.
2. I believe the PR is addressing the consequences of a design issue instead of addressing the design issue itself, which is why I am considering a different approach, although the PR raised some insightful ideas.

Let me expand on that,

So we have RabbitMQ, Redis and Memcached vendors.
RabbitMQ vendor provides a broker component.
Redis vendor provides both broker and backend components.
Memcached vendor provides a backend component.

*Component is a docker container and a node controller.

pytest-celery does not explicitly install any of the libraries required for all of the provided vendors because it relies on the celery extras to provide the matching dependencies, so the test environment and the worker container will install the exact same versions.

Where is the design problem then IMHO? API, interface.
You expect pip install pytest-celery to provide everything for the provided API, but it doesn’t, because it depends on the user to also install the matching celery extras, again, to maintain sync between the test/tested environments.

This is not intuitive, nor the best implementation for the given goal, I agree.

So, let me ask what would you say about this interface:

• pytest-celery - Vanilla install. No Redis, no Memcached dependencies (zero automatic vendors).
• pytest-celery[celery] - Plugin + celery[redis,pymemcache]
• pytest-celery[redis] - Plugin + latest redis
• pytest-celery[memcache] - Plugin + latest pymemcache
• pytest-celery[redis,memcache] - Plugin + latest redis+ latest pymemcache.

Also maybe if it’s not too much,

• pytest-celery[brokers] - Plugin + latest redis
• pytest-celery[backends] - Plugin + latest redis + latest pymemcache

Which can all be bundled alongside the celery extra, e.g., pytest-celery[celery,brokers], then configure your test environment to disable the backend to avoid import errors.

---

Lastly, this means the error handling needs to be developed for better package handling/error messages.

I would like to attempt a broad and more thorough implementation of the fix, which might be out of the scope of your PR (might require some deeper/inner design changes in other places) - would it be alright with you @woutdenolf if I close the PR then and work on the implementation alongside our conversation here?

EDIT:
@woutdenolf

As I understand, the smoke tests use an editable install of celery.

This indeed follows the explanation above, where pytest-celery comes “vanilla,” and the smoke tests install the dependencies explicitly, just off the source code.

COPY --chown=test_user:test_user . /celery
RUN pip install --no-cache-dir --upgrade \
    pip \
    -e /celery[redis,pymemcache] \

[woutdenolf]

I still argue that the pytest-celery should NOT INSTALL celery by default due to packages sensitivity.

I'll have to see a real use case to be convinced. Anyway, the projects that use pytest-celery most likely have celery as a dependency so I won't insist. Still it seems an unusual design choice for a pytest plugin.

So, let me ask what would you say about this interface:
Also maybe if it’s not too much

Lets think of pytest-celery as any other python project. We have these import statements

from celery import Celery
from kombu import Connection
from redis import StrictRedis
import memcache

So all of these are dependencies, optional or required but dependencies regardless. When none of the optional dependencies are installed, these import statements should not fail imo

import pytest_celery         # if this fails, you cannot use pytest-celery without the optional dependencies
import pytest_celery.plugin  # if this fails, you cannot use pytest without the optional dependencies

I believe the real design issue is that everything is imported in the main __init__.py file of the project. This is why nothing can actually be optional at the moment.

In my PR I captured the ImportError from all optional dependencies (redis and memcache, in which case celery and kombu are still required) and silently ignored them. A better solution (I believe that is what you were hinting at) would be to not import any of the optional dependencies when importing

import pytest_celery
import pytest_celery.plugin

Then when the tests of other projects actually trying to use the optional dependencies, an exception gets raised at that moment.

Then in terms of the project extra's. Again lets think of the projects we import: celery, kombu, redis and memcache. If all of them are optional I'd propose to do what any python project would do

pip install pytest-celery[celery,redis,memcache]  # strictly speaking rabbitmq needs to be in here as well which we use kombu for

In projects with many optional dependencies I often add the "full" extra to install all of them (which is the equivalent of what "celery" currently does)

pip install pytest-celery[full]

That being said I'm also ok not having any of the individual optional dependencies and only keep "full" or "celery" as it is now called. So basically leave things the way they are (perhaps fixing the celery version thing). The important part is not breaking pytest when optional dependencies are missing and only raise an import error when a project actually tries to use them.

would it be alright with you if I close the PR then and work on the
implementation alongside our conversation here?

Sure. In the end if I can use pytest-celery without having to install python-memcache my immediate problem is solved.

[Nusnus]

@woutdenolf

I’ve decided to take a different angle in approaching this issue.

first,

Sure. In the end if I can use pytest-celery without having to install python-memcache my immediate problem is solved.

second,

I'll have to see a real use case to be convinced. Anyway, the projects that use pytest-celery most likely have celery as a dependency so I won't insist. Still it seems an unusual design choice for a pytest plugin.

When I re-review this subject from your perspective, I start to make sense of what’s wrong with my reasoning.

1. “Premature Optimization” - The motivation for Do not install Celery by default, use extra [celery] instead #172 was an assumption no one complained about. You said it yourself, "I'll have to see a real use case to be convinced.”; you are right.
2. “Design Flaw” - That being said, we did find a real issue. The vendors dependencies should be better managed, and right now the existing functionality of the plugin expects full installation (redis & memcache in this case) due to the defaults.py lists.
3. As we can see, the immediate solution is not fully agreed upon, with probably additional caveats to discuss. This causes the fix to be delayed or forcing an unbaked fix which is not recommended either.

So, to accommodate the above reasoning into priorities, I’ve decided to revert the celery extra altogether in #221.
This will allow me to release v1.0.0b3, including all recent additions, and you to overcome the immediate block you have from the missing memcache dependency, which is what this bug report is about in the first place.

Lastly, the design flaw can be solved separately, making sure the problem this discussion raised are not repeating, allowing focusing at each issue at a time, where it hurts more first :)