Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

python.raise_exception() docs give wrong signature #1261

Open
rsyring opened this issue Jan 9, 2025 · 3 comments
Open

python.raise_exception() docs give wrong signature #1261

rsyring opened this issue Jan 9, 2025 · 3 comments

Comments

@rsyring
Copy link

rsyring commented Jan 9, 2025

Describe the bug

I believe the example from the docs for this function is wrong:

python.raise_exception(
    name="Raise NotImplementedError exception",
    exception=NotImplementedError,
    message="This is not implemented",
)

It produces the error:

    [@local] Unexpected error in Python callback: TypeError('NotImplementedError() takes no keyword arguments',)

It should actually be:

python.raise_exception(
    NotImplementedError,
    "This is not implemented",
    name="Raise NotImplementedError exception",
)

Meta

    System: Linux
      Platform: Linux-6.8.0-51-generic-x86_64-with-glibc2.39
      Release: 6.8.0-51-generic
      Machine: x86_64
    pyinfra: v3.1.1
      click: v8.1.3
      configparser: v7.1.0
      distro: v1.9.0
      gevent: v24.11.1
      jinja2: v3.1.2
      packaging: v24.2
      paramiko: v3.5.0
      pytest: v7.4.0
      pytest: v7.4.0
      python-dateutil: v2.8.2
      pywinrm: v0.5.0
      setuptools: v75.8.0
      typeguard: v4.4.1
      typing-extensions: v4.12.2
      wheel: v0.40.0
    Executable: /home/rsyring/.local/pipx/venvs/kilo/bin/pyinfra
    Python: 3.11.7 (CPython, GCC 13.2.0)
@simonhammes
Copy link
Contributor

I can reproduce the issue.

One downside of your proposed fix is the inconsistency with all the other operations which allow the global name argument as the first argument :/

But I don't know how this can be fixed apart from hardcoding which params are passed to the exception constructor...

@rsyring
Copy link
Author

rsyring commented Jan 15, 2025

Thanks for getting back to me on this.

One downside of your proposed fix...

Sorry for the confusion. I wasn't actually proposing the fix. I was just documenting what works:

python.raise_exception(
    NotImplementedError,
    "This is not implemented",
    name="Raise NotImplementedError exception",
)

That's how I had to use it for it to work without an error. I'm not saying that should be how it works in the future. If you need to change the code so the call works like the docs example, that seems just fine to me. It would just be a backwards incompatible change.

@simonhammes
Copy link
Contributor

I wasn't actually proposing the fix. I was just documenting what works:

Ah, I'm sorry, I misread the issue description.

If you need to change the code so the call works like the docs example, that seems just fine to me. It would just be a backwards incompatible change.

Hmm. Imo the incorrect docs example should be fixed; the inconsistency could always be fixed later.
Mind creating a PR to fix the docs?

@Fizzadar Do you have any ideas how the kwargs handling could be improved?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants