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

IReactorTCP.connectTCP incorrectly documents its 'host' parameter's type #10251

Closed
twisted-trac opened this issue Aug 9, 2021 · 4 comments
Closed

Comments

@twisted-trac
Copy link

twisted-trac commented Aug 9, 2021

richvdh's avatar @richvdh reported
Trac ID trac#10251
Type defect
Created 2021-08-09 10:46:30Z
Branch https://github.com/twisted/twisted/tree/10251-connecttcp-host-type

IReactorTCP.connectTCP claims that its host parameter should be a bytes.

However, this is inconsistent with usage elsewhere in Twisted, and furthermore the implementation in PosixReactorBase assumes otherwise. (It will work with bytes, but other documentation suggests it expects a str.)

Searchable metadata
trac-id__10251 10251
type__defect defect
reporter__richvdh richvdh
priority__normal normal
milestone__None None
branch__10251_connecttcp_host_type 10251-connecttcp-host-type
branch_author__ 
status__closed closed
resolution__fixed fixed
component__core core
keywords__review review
time__1628505990213223 1628505990213223
changetime__1632082226837188 1632082226837188
version__None None
owner__Tom_Most__twm_____ Tom Most <twm@...>

@twisted-trac
Copy link
Author

twisted-trac commented Aug 9, 2021

richvdh's avatar @richvdh commented

Verdict from the mailing list: this should be treated as a simple bug in the interface definition, and fixed.

Any IReactorTCP implementation that strictly implemented the interface-as-written by only accepting bytes wouldn't work, so there isn't any compatibility hazard.

@twisted-trac
Copy link
Author

twisted-trac commented Sep 19, 2021

twm's avatar @twm set owner to @twm
@twm set status to assigned

@twisted-trac
Copy link
Author

twisted-trac commented Sep 19, 2021

twm's avatar @twm removed owner
@twm set status to new

PR: #1664

@twisted-trac
Copy link
Author

twisted-trac commented Sep 19, 2021

twm's avatar @twm set owner to @twm
@twm set status to closed

In changeset ec7445d

#!CommitTicketReference repository="" revision="ec7445d6361ce04424b3570a81263dc6760f15de"
Merge pull request #1664 from twisted/10251-connecttcp-host-type

Author: twm
Reviewer: adiroiban
Fixes: ticket:10251

Correct the IReactorTCP.connectTCP host parameter type annotation to match
real-world implementations as discussed on the mailing list.

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

No branches or pull requests

2 participants