Skip to content

gh-56915: Fix documented timeout default in ftplib - #158630

Open
Monstertov wants to merge 3 commits into
python:mainfrom
Monstertov:gh-56915-ftplib-poplib-timeout-docs
Open

Monstertov wants to merge 3 commits into
python:mainfrom
Monstertov:gh-56915-ftplib-poplib-timeout-docs

Conversation

@Monstertov

@Monstertov Monstertov commented Oct 2, 2026 •

Copy link
Copy Markdown

the FTP.connect docs and the FTP class docstring both describe the wrong default for timeout:

  • FTP.connect uses a private sentinel. omitting timeout preserves the instance's timeout, initially set by the constructor.
  • FTP defaults to socket._GLOBAL_DEFAULT_TIMEOUT, so sockets use the global default timeout. passing None explicitly disables the timeout.

this fixes the FTP.connect default text and the FTP class docstring. the timeout=None signatures in the docs are left as is for now, see the discussion above.

docs and docstring only, no behaviour change, so i think this can skip news.

quick check of the socket timeout after a successful connection:

socket.setdefaulttimeout(7.0)
FTP().connect(host, port)              # sock.gettimeout() -> 7.0
FTP(timeout=None).connect(host, port)  # sock.gettimeout() -> None
FTP(timeout=3).connect(host, port)     # sock.gettimeout() -> 3.0

the docs showed timeout=None for FTP, FTP.connect, FTP_TLS and POP3_SSL,
but the code defaults to the global socket timeout. passing None
disables the timeout instead. use the [, timeout] form like smtplib and
http.client, and say that FTP.connect keeps the constructor timeout.
@Monstertov
Monstertov requested review from a team and giampaolo as code owners October 2, 2026 23:38
@python-cla-bot

python-cla-bot Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app

bedevere-app Bot commented Oct 2, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@read-the-docs-community

read-the-docs-community Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

@bitdancer

Copy link
Copy Markdown
Member

The [, timeout] style is, as far as I can tell, old. It probably predates _GLOBAL_DEFAULT_TIMEOUT. I think using it is incorrect: the arguments are keyword arguments with values, not optional positional arguments or required keywords.

What I recommend here is to revisit #91225 in light of the introduction of sentinel support in Python. That doesn't fix the inconsistency in the docs for the maintenance versions, but I don't think the change suggested in this PR is the correct approach to fixing that. Any suggestion for a doc-only fix should also be applied to those other arguably incorrect (and I believe legacy) uses of [ timeout,].

@Monstertov

Copy link
Copy Markdown
Author

thanks, agreed. [, timeout] is wrong here, especially after * in FTP_TLS and POP3_SSL.

as i see it, two options:

  1. docs-only, backportable: use timeout=GLOBAL_DEFAULT like socket.create_connection does, and apply it everywhere [, timeout] is used now (smtplib, http.client, urllib.request, poplib).
  2. main only: make _GLOBAL_DEFAULT_TIMEOUT a real sentinel (socket._GLOBAL_DEFAULT_TIMEOUT being an object() makes for ugly docstrings, can be better #91225), and give FTP.connect its own sentinel instead of -999.

the FTP.connect default text and the class docstring are wrong either way. should i keep only those here and drop the signature changes, then follow up with whichever option you prefer?

@bitdancer

Copy link
Copy Markdown
Member

I think timeout=GLOBAL_DEFAULT is also misleading since GLOBAL_DEFAULT doesn't exist in any namespace.

So let's go with the non-signature fixes only here, and fix the signatures after adding the sentinels..

Revert the signature-notation changes per review; the proper signature
fix is deferred to a follow-up that adds real timeout sentinels. This
keeps the FTP.connect default-timeout text and the FTP class docstring
corrections.
@Monstertov

Copy link
Copy Markdown
Author

@bitdancer done. dropped the signature changes and kept only the non-signature fixes: the FTP.connect default text and the FTP class docstring. the [, timeout] edits are reverted to timeout=None as on main.

i'll do the signatures in a follow-up after the sentinels land (#91225).

@Monstertov Monstertov changed the title gh-56915: Fix documented timeout default in ftplib and poplib gh-56915: Fix documented timeout default in ftplib Oct 5, 2026
@Monstertov

Copy link
Copy Markdown
Author

the MSan failure (test_os.test_posix.test_fexecve) is unrelated to this change, it's the same failure as gh-158868.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants