Troubles to understand the new updates on Pluggy

[Tiarles]

Hi everybody.

I am having issues updating my pytest plugin after pluggy==1.0.0 updates. Accordingly, with the documentation all the hooks decorated with hookwrapper=True need to be written as a generator because, without this, it would cause an error later. In my application, the hook wrappers were always a generator but the PluginValidationError is still being thrown. I isolated the if statement (Code snippet 1) for that Error and my hook wrapper passes, but not in the pluggy (on .\py38\lib\site-packages\pluggy\_manager.py file) implementation which always falls on the exception:

# Code snippet 1: Lines 341 to 358 from pluggy\_manager.py (version pluggy==1.3.0)
if (
    hookimpl.wrapper or hookimpl.hookwrapper
) and not inspect.isgeneratorfunction(hookimpl.function):
    raise PluginValidationError(
        hookimpl.plugin,
        "Plugin %r for hook %r\nhookimpl definition: %s\n"
        "Declared as wrapper=True or hookwrapper=True "
        "but function is not a generator function"
        % (hookimpl.plugin_name, hook.name, _formatdef(hookimpl.function)),
    )

Also, the changelog for the version 1.0.0 is very confusing. First, the exception name is PluginValidationError and not PluggyValidationError as is written there. Second is that the example to properly make the changes has functions that are not used and others that have no reference in the documentation for it (Code snippet 2).

# Code snippet 2.1: Changed example from Pluggy CHANGELOG version 1.0.0

def my_hook_real_implementation(arg):  # Function created but not used on the documenation
    print("before")
    yield
    print("after")


@hookimpl(hookwrapper=True)
def my_hook(arg):
    return my_hook_implementation(arg)  # Not declared function

change it to use yield from instead:

# Code snippet 2.2: Changed example from Pluggy CHANGELOG version 1.0.0
@hookimpl(hookwrapper=True)
def my_hook(arg):
    yield from my_hook_implementation(arg)  # Not declared function

I considered in this example that my_hook_real_implementation is the function my_hook_implementation but still my hook wrap falls in the same exception.

I comment the Code Snippet 1 in my enviroment with the same version of Pluggy and the execution happened with no errors.

I don`t know what is needed to fix this issue on my side, maybe a more explanatory example could be provided or maybe pluggy needs updates.

Let me know which more data is needed or what I can do it to help. Since my plugin code is from the repository of the company that I work, I didn`t provided the source code here but we can arrange some way to get more data and a reproduceable example.

[RonnyPfannschmidt]

The basic gist is that before we didn't check if the function was an actual generator,

So a normal function that returned one would work by accident

With the introduction of new style wrappers however validation was added

The easiest fix is to switch from return to yield from

That changes the function from a normal one that returns a generator into a generator function

[bluetech]

Thanks for reporting the changelog errors, will be fixed by #470.

[Tiarles]

I found what was causing the issue on my side. I implemented everything as specified in the documentation, but my plugin is not able to make the condition:

not inspect.isgeneratorfunction(hookimpl.function)

be

not (True)

And then not throw the PluginValidationError exception because my hookimpl.function is a type <class '_cython_3_0_0.cython_function_or_method'>. Since we Cythonize our plugin, and the source version works just as expected.

Should I proceed with my investigation here or open a new issue? I believe I can collect more info in the next weeks. If I find some time I can also contribute to it.

[bluetech]

And then not throw the PluginValidationError exception because my hookimpl.function is a type <class '_cython_3_0_0.cython_function_or_method'>. Since we Cythonize our plugin, and the source version works just as expected.

Just to be clear, you're saying that when you cythonize a generator function, inspect.isgeneratorfunction returns False for it? Sounds pretty strange to me. Maybe file a bug with Cython asking them to set the generator flag on the function?

[Tiarles]

I will take a closer look at that today. But yes, the inspect.isgeneratorfunction is returning False for a cythonized generator. I will check if maybe has another way to check that when the generator is Cythonized, maybe a method from Cython or if is the case that you mentioned and write for them about it.

[bluetech]

Seems like there is an issue in Cython about it: cython/cython#4888

I think adding some Cython-specific workaround would be fine, but I'm hesitant to drop the check entirely due to a Cython bug.

[dongfangtianyu]

Hi, I have paid attention to and explored this issue, and here I would like to share some of my findings.

Cause

Following the cythonization process, the sequence

inspect.isgeneratorfunction -> _has_code_flag -> isfunction

return a False.

Reproduce

docker run --rm -it python:3.10.5-alpine  sh -c "pip install pytest-result-log==1.2.2.dev0 && pytest"

Occurrence

python <= 3.10.5

Resolution

The issue has been addressed in the upstream Python repository: https://github.com/python/cpython/pull/94461/files

if not (isfunction(f) or _signature_is_functionlike(f)):

It is recommended to update Python to versions released after August 1, 2022, such as:

• 3.9.14
• 3.10.6+
• 3.11+

Verify

// docker-compose.yaml
services:
  python310:
    image: python:3.10.6-alpine
    command: sh -c "pip install pytest-result-log==1.2.2.dev0 && pytest "
    working_dir: /app

  python311:
    image: python:3.11.0-alpine
    command: sh -c "pip install pytest-result-log==1.2.2.dev0 && pytest "
    working_dir: /app
  python312:
    image: python:3.12.0-alpine
    command: sh -c "pip install pytest-result-log==1.2.2.dev0 && pytest "
    working_dir: /app

For older versions of Python, can also achieve a similar effect by using monkey patching to modify inspect._has_code_flag.

[Tiarles]

I wrote an issue on Cython Github page here, regarding the quoting:

Seems like there is an issue in Cython about it: cython/cython#4888

I think adding some Cython-specific workaround would be fine, but I'm hesitant to drop the check entirely due to a Cython bug.

[Tiarles]

So, I finished the discussion with the Cython devs (link), and has nothing that they can do. Possible solutions are to pluggy changes how to inspect those generators when they are Cythonized, or we just assume that the problem exists and is known for some older Python versions.

We can also use the insights published in this quote.

[RonnyPfannschmidt]

we can work around a old cpython bug within reason

[Tiarles]

Yeah, I will wait until we deprecate support for Python 3.8, and use these newest Python versions of 3.9, 3.10, and 3.11 to cythonize our API. The question is if pluggy will work in older versions of the Python distributions when the pytest plugins are cythonized in a newer one. I don't think so but I can try that in a few days.