NotificationsYou must be signed in to change notification settings
Fork32k
Star67.3k

gh-128319: Add missing server class attributes to socketserver docs#128320

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

stephen-hansen wants to merge3 commits intopython:main

base:main

Choose a base branch

fromstephen-hansen:docs-socketserver-missing-attributes

Open

gh-128319: Add missing server class attributes to socketserver docs#128320

stephen-hansen wants to merge3 commits intopython:mainfromstephen-hansen:docs-socketserver-missing-attributes

+29 −7

Conversation

Copy link

Contributor

stephen-hansen commentedDec 28, 2024•
edited by github-actionsbot
Loading

Just doing some cleanup of the socketserver docs regarding class attributes as mentioned in the linked issue.

Moveaddress_family attribute from instance attributes to class attributes.
Documentallow_reuse_port.
Documentmax_packet_size (note UDP only).
Add note torequest_queue_size to denote TCP only.

Issue:Some server class attributes are missing in socketserver docs #128319

📚 Documentation preview 📚:https://cpython-previews--128320.org.readthedocs.build/

Add missing server class attributes to socketserver docs

a55bfff

bedevere-appbot mentioned this pull request

Dec 28, 2024

Some server class attributes are missing in socketserver docs#128319

Open

bedevere-appbot added docs

Documentation in the Doc dir

skip news awaiting review labels

Dec 28, 2024

ZeroIntensity added needs backport to 3.12

only security fixes

needs backport to 3.13bugs and security fixes labels

Dec 29, 2024

ZeroIntensity reviewed

Dec 29, 2024

View reviewed changes

Copy link

Member

ZeroIntensity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for doing this :)

Doc/library/socketserver.rst OutdatedShow resolvedHide resolved

picnixz reviewed

Dec 29, 2024

View reviewed changes

Doc/library/socketserver.rstShow resolvedHide resolved

Doc/library/socketserver.rst OutdatedShow resolvedHide resolved

stephen-hansenand others added2 commits

December 29, 2024 11:32

fix max_packet_size spacing

eee013c

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Apply suggested review comments

62c120a

reactive-firewall reviewed

Apr 9, 2025

View reviewed changes

Copy link

reactive-firewall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍🏻 LGTM! With conditions (see knit-picky inline comment)

🙇 Thanks@stephen-hansen; These document drafts have already been useful to me.

Doc/library/socketserver.rst

Comment on lines +311 to +314

		The maximum amount of data (in bytes) to receive at once. If a packet is
		too large for the buffer, then the excess bytes beyond :attr:`max_packet_size`
		are discarded. The default value is usually ``8192``, but this can be
		overridden by subclasses.

Copy link

reactive-firewallApr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM! With conditions:

Please consider clarifying if the term "data" here, means without IP/UDP headers with something like:

Suggested change

	The maximum amount of data (in bytes) to receive at once. If a packet is
	too large for the buffer, then the excess bytes beyond:attr:`max_packet_size`
	are discarded. The default value is usually ``8192``, but this can be
	overridden by subclasses.
	The maximum amount of data (in bytes) to receive at once. If a packet's
	payload (excluding headers) istoo large for the buffer, then the excess
	bytes beyond:attr:`max_packet_size`are discarded. The default value
	is usually ``8192``, but this can beoverridden by subclasses.

or if this really is the literal maximum packet size:

Suggested change

	The maximum amount of data (in bytes) to receive at once. If a packet is
	too large for the buffer, then the excess bytes beyond:attr:`max_packet_size`
	are discarded. The default value is usually ``8192``, but this can be
	overridden by subclasses.
	The maximum amount of data (in bytes) to receive at once. If a packet's
	payload (including headers) istoo large for the buffer, then the excess
	bytes beyond:attr:`max_packet_size`are discarded. The default value
	is usually ``8192``, but this can beoverridden by subclasses.

Rational:
Omission of any mention of header when talking about a "packet" being "too large" is somewhat misleading to the reader.
not sure if going further is relevant to the python side, but for convenience sake I'll mention, the typical 20-byte IPV4 header (RFC-791 gives 20-byte base header, 40-byte maximum options) and the required 8-byte header assuming UDP (RFC-768).

Hopefully this helps.

Additional considerations (TL;DR):

MTU, and fragmentation boundaries are possible reasons to want to change the default (eg. expected max burst count * max packet size (including headers) on network path with values of5 * (1500 - 20 - 40 - 8) =7160 so the default is somewhat larger than the similar stream sockets'request_queue_size default)
UDP max packet size (after headers) is a possible (albeit unlikely) reason for increasing the size well beyond the default of8192