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

twisted uses deprecated three-argument form of generator.throw #12026

Closed
AdamWill opened this issue Oct 28, 2023 · 4 comments · Fixed by #12027
Closed

twisted uses deprecated three-argument form of generator.throw #12026

AdamWill opened this issue Oct 28, 2023 · 4 comments · Fixed by #12027
Labels

Comments

@AdamWill
Copy link
Contributor

AdamWill commented Oct 28, 2023

Describe the incorrect behavior you saw
src/twisted/python/failure.py does return g.throw(self.type, self.value, self.tb). This is deprecated in Python 3.12: python/cpython#96348 , https://docs.python.org/3.12/whatsnew/3.12.html#deprecated . This is a particular problem for https://github.com/fedora-infra/fedora-messaging consumers - they print a deprecation warning about once a second.

Describe how to cause this behavior
Try using twisted with Python 3.12, in a way that hits this codepath (I'm not sure precisely how to do that other than "use a fedora-messaging consumer", sorry). You get a deprecation warning:

Oct 27 23:11:08 openqa-lab01.iad2.fedoraproject.org fedora-messaging[172628]: /usr/lib/python3.12/site-packages/twisted/python/failure.py:518: DeprecationWarning: the (type, exc, tb) signature of throw() is deprecated, use the single-arg signature instead.
Oct 27 23:11:08 openqa-lab01.iad2.fedoraproject.org fedora-messaging[172628]:   return g.throw(self.type, self.value, self.tb)

Describe the correct behavior you'd like to see
No deprecation warning.

Testing environment
Python 3.12 on Fedora 39, don't think anything else is relevant. The code is still using the deprecated form on current git trunk.

Additional context
I would send a pull request changing this, but I'm unclear on whether it's safe to just use the modern one-argument form on Python 3.8.

@AdamWill AdamWill added the bug label Oct 28, 2023
@glyph
Copy link
Member

glyph commented Oct 28, 2023

I would send a pull request changing this, but I'm unclear on whether it's safe to just use the modern one-argument form on Python 3.8.

Would it be possible to prepare two PRs, one which does the one-argument form, and one which suppresses the deprecation so we don't get the log spam until we can drop 3.8? It would be nice to get it churning through CI in the meanwhile so we can land it quickly, whichever approach works. (And if it's not actually safe on 3.8, presumably CI will tell us fairly loudly)

@AdamWill
Copy link
Contributor Author

AdamWill commented Oct 28, 2023

well, possibly CI wouldn't catch it - even with the three-argument form, the last two arguments are optional, so it's not just flat-out invalid to pass it only one argument on 3.8.

It's just that, at least as documented, if you pass it one argument on 3.8, it expects that argument to be an exception type, not an instance. Presumably, whenever the one-argument-that's-an-instance signature was actually introduced, it was set up so that if you pass it one argument it checks whether it's a type or an instance and behaves appropriately.

But at least as documented, doing generator.throw(exception_instance) on 3.8 is syntactically valid but the actual consequences of doing it are unclear...I guess it would depend what those consequences are, and whether the tests are sophisticated enough to catch it if the consequences are wrong?

It's a bit late here now, so I think I'll maybe see if we get any comments on my questions in python/cpython#96348 today or tomorrow...

AdamWill added a commit to AdamWill/twisted that referenced this issue Oct 28, 2023
In Python 3.12, the 3-arg signature of generator.throw() is
deprecated, you're only supposed to use the 1-arg signature
where you pass only an exception instance. I *think* this is the
right thing to pass in this case.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
AdamWill added a commit to AdamWill/twisted that referenced this issue Oct 28, 2023
In Python 3.12, the 3-arg signature of generator.throw() is
deprecated, you're only supposed to use the 1-arg signature
where you pass only an exception instance. I *think* this is the
right thing to pass in this case.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
AdamWill added a commit to AdamWill/twisted that referenced this issue Oct 28, 2023
The three-arg form of generator.throw() was deprecated in Python
3.12. However, we're not sure if the modern one-arg form works
on Python 3.8, which is still supported, so instead of changing
it right now, this just suppresses the warning.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
@AdamWill
Copy link
Contributor Author

well, I guess sending the PRs can't hurt, so there they are.

@glyph
Copy link
Member

glyph commented Oct 28, 2023

Thanks, much appreciated!

AdamWill added a commit to AdamWill/twisted that referenced this issue Dec 27, 2023
The three-arg form of generator.throw() was deprecated in Python
3.12. However, we're not sure if the modern one-arg form works
on Python 3.8, which is still supported, so instead of changing
it right now, this just suppresses the warning.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
AdamWill added a commit to AdamWill/twisted that referenced this issue Dec 27, 2023
In Python 3.12, the 3-arg signature of generator.throw() is
deprecated, you're only supposed to use the 1-arg signature
where you pass only an exception instance. I *think* this is the
right thing to pass in this case.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants