Opened 3 years ago

Closed 3 years ago

#7702 enhancement closed fixed (fixed)

Remove twisted.application.internet.UDPClient.

Reported by: Tom Prince Owned by: Glyph
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/remove-udpclient-7702
branch-diff, diff-cov, branch-cov, buildbot
Author: glyph

Description

It was deprecated in #6468 (13.1), since it was broken. The deprecation period has ended, and the code doesn't work.

Attachments (2)

removal_7702.patch (1.9 KB) - added by Eeshan Garg 3 years ago.
removal_7702_2.patch (3.3 KB) - added by Eeshan Garg 3 years ago.

Download all attachments as: .zip

Change History (10)

Changed 3 years ago by Eeshan Garg

Attachment: removal_7702.patch added

comment:1 Changed 3 years ago by Eeshan Garg

Keywords: review added

Hello!

I just added a patch for this(removal_7702.patch​). This patch removes all references to twisted.application.internet.UDPClient from the codebase.

If something needs to be improved or if I did something wrong, please let me know. I would love to work on it. :-)

Thanks & regards, Eeshan Garg

comment:2 Changed 3 years ago by Adi Roiban

Keywords: review removed
Owner: set to Eeshan Garg

Hi,

Thanks for the review.

All this post-mortem deprecation was added in #6469.

Please check the merge commit and remove all lines introduced there https://github.com/twisted/twisted/commit/70cc31a4b19e593c63e6e5da3d57ecae5f8ef5cf#diff-e1da3f98608a56680f93e0d2f0aa8b0dR23

You should also remove the import lines

+from twisted.python.deprecate import deprecatedModuleAttribute
+from twisted.python.versions import Version

Then news line is a bit missleading. This just removed the deprecation warning, introduced in 13.1 ... but the real code was deprecated/removed in version 10

Thanks!

comment:3 Changed 3 years ago by Tom Prince

After applying this patch, UDPClient is still defined. (Note that that string doesn't appear in the source at the point of definition).

comment:4 Changed 3 years ago by Adi Roiban

As discussed over IRC, the UDPClient class is created at runtime by combining 'UDP' string with 'Client' string. I think that the patch for this branch should prevent this combination and raise ImportError

In case someone else is confuses (like I was) here is the code: https://github.com/twisted/twisted/blob/trunk/twisted/application/internet.py#L207-L209

Thanks!

Changed 3 years ago by Eeshan Garg

Attachment: removal_7702_2.patch added

comment:5 Changed 3 years ago by Eeshan Garg

Keywords: review added
Owner: Eeshan Garg deleted

Hi!

I just submitted a new patch(removal_7702_2.patch) for this.

There were two tests that failed because of removing UDPClient. I modified the tests to exclude UDPClient.

I ran the tests before and after applying this patch. All of the tests passed.

You can no longer import twisted.application.internet.UDPClient now. :)

Thanks tom.prince, adiroiban for your guidance! If there is something I got wrong, please re-assign this ticket to me, I would love to work on this and close this ticket. :-)

Regards, Eeshan Garg​

comment:6 Changed 3 years ago by Glyph

Author: glyph
Branch: branches/remove-udpclient-7702

(In [43726]) Branching to remove-udpclient-7702.

comment:7 Changed 3 years ago by Glyph

Keywords: review removed
Owner: set to Glyph

Thanks for submitting this patch, eeshangarg! Looks like everything is in order. Buildbot is happy. I'll land this now.

comment:8 Changed 3 years ago by Glyph

Resolution: fixed
Status: newclosed

(In [43728]) Merge remove-udpclient-7702: remove UDPClient

Author: eeshangarg

Reviewer: glyph

Fixes: #7702

Remove twisted.application.internet.UDPClient, deprecated since Twisted 13.1.0.

Note: See TracTickets for help on using tickets.