-
Notifications
You must be signed in to change notification settings - Fork 39
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
Use return
instead of twisted.defer.returnValue
#2955
base: master
Are you sure you want to change the base?
Use return
instead of twisted.defer.returnValue
#2955
Conversation
returnValue has been deprecated in twisted 24.7.0
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2955 +/- ##
==========================================
- Coverage 56.35% 56.34% -0.02%
==========================================
Files 603 603
Lines 43849 43838 -11
Branches 48 48
==========================================
- Hits 24713 24702 -11
Misses 19124 19124
Partials 12 12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
In the last week when running the tests I got a lot of a similar warning:
I replaced all occurrences of
defer.returnValue
withreturn
.Reference: https://github.com/twisted/twisted/blob/HEAD/NEWS.rst#deprecations-and-removals, twisted/twisted#9930