Use return instead of twisted.defer.returnValue#2955
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2955 +/- ##
==============================
==============================
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
While I'm really glad to see Twisted embrace newer Python constructs by deprecating their old kludges (that, in all fairness, were implemented to enable these kinds of programming paradigms under older Pythons), I think this approach should not be the immediate one.
Although the deprecation warnings do not actually break anything in NAV, they will be quite annoying, and so we have to make a decision about locking the Twisted version requirement.
Currently, NAV claims to work with at least version 23.8. If we make these changes, NAV will no longer work with Twisted 23, and we must upgrade to Twisted 24. It's either that, or lock to Twisted 23 in stable versions and make this change later.
I also think we should investigate the other changes mentioned in relation to this: The fact that the inlineCallbacks decorator might change or become deprecated in favor or Python's built-in co-routines. This would affect a lot of the ipdevpoll codebase as well.
There's also the slightly unrelated fact that a lot of ipdevpoll code was started even before inlineCallbacks appeared, and as such are purely callback-oriented. These parts of the codebase might stand to be refactored as well.
My dream is that we eventually will migrate the codebase to asyncio - the only reason we are sticking with Twisted is because of the inherited twistedsnmp interface of pynetsnmp. I believe it is quite possible to have Twisted and asyncio to co-exist these days. If the codebase could be mostly asyncio-oriented, with adaptations to keep pynetsnmp, that would be nice. There's also the option to rewrite pynetsnmp-2 to asyncio, since we've already forked it.
I.e. I want to keep this, but not merge it just yet.
|
Link for future reference: twisted/twisted#12239 |
Twisted 24 introduces many deprecation warnings, and we want to avoid them. PR #2955 introduces a fix, of sorts, but this would require Twisted 24 and above, so it has been put on ice for now.
d959a80 to
8e573ea
Compare
|
I've rebased this on the latest master and added a commit to change NAV's requirement to |
|
The above mentioned Twisted PR that changes this behavior seems to suggest that there might be problems using |
The main PR description mentions this without detail, but delving into the documentation the PR changes, it seems this only applies to functions that are rewritten from using This will be a problem for a different PR. |
lunkwill42
left a comment
There was a problem hiding this comment.
As mentioned, I rebased this onto the latest master. The only relevant change from the first iteration, I guess, was to the recently updated paloaltoarp.py plugin in ipdevpoll.
returnValue has been deprecated in twisted 24.7.0
From Twisted 24.7, `twisted.internet.defer.returnValue()` has been deprecated and can be replaced by using Python's built-in `return`. We have done so, so there is no going back.
8e573ea to
8450d79
Compare
|
In the last week when running the tests I got a lot of a similar warning:
I replaced all occurrences of
defer.returnValuewithreturn.Reference: https://github.com/twisted/twisted/blob/HEAD/NEWS.rst#deprecations-and-removals, twisted/twisted#9930