FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-60856: Be explicit about localhost for socket.getfqdn by slateny · Pull Request #93451 · python/cpython · GitHub

/ cpython Public

gh-60856: Be explicit about localhost for socket.getfqdn - #93451

Open
slateny wants to merge 5 commits into
python:mainfrom
slateny:s/60856
Open

gh-60856: Be explicit about localhost for socket.getfqdn#93451
slateny wants to merge 5 commits into
python:mainfrom
slateny:s/60856

Conversation

slateny commented Jun 3, 2022
edited
Loading

Copy link
Copy Markdown
Contributor

bedevere-bot added docs Documentation in the Doc dir awaiting review labels Jun 3, 2022
slateny marked this pull request as draft June 3, 2022 06:25

slateny commented Jun 3, 2022

Copy link
Copy Markdown
Contributor Author

Noticed there's duplicate wording in the last sentence, will change the PR for that

slateny marked this pull request as ready for review June 3, 2022 06:48
iritkatriel requested a review from ambv October 4, 2022 22:32
Comment thread Doc/library/socket.rst Outdated
Return a fully qualified domain name for *name*. If *name* is omitted or empty,
it is interpreted as the local host. To find the fully qualified name, the
Return a fully qualified domain name for *name*. If *name* is empty or equal to
``'0.0.0.0'``, the hostname from :func:`gethostname` is returned.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is it so? Looking at the sources, it tries to return one of names returned by gethostbyaddr(gethostname()).

The new wording also leaves unclear what getfqdn() returns.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The source/behavior was changed in #100375 after this PR was opened, so wording definitely does need updating.

Regarding the unclear new wording, could you expand on that? My interpretation is that previously the entire paragraph needs to be read through before the return of getfqdn(), but now it's right at the top: If *name* is empty ...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Now it starts from the rare corner case (why would anyone pass an empty name to getfqdn()?), but the documentation for getfqdn() without arguments is removed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

You are very correct, leaving out the empty argument is my mistake. What do you think about this instead?

Return a fully qualified domain name for *name*.
To find the fully qualified name, ...
...
... it is returned unchanged.
If *name* is empty, or equal to "0.0.0.0," "::", or "",
the hostname from :func:`gethostname` is returned.

There's also an option of changing getfqdn([name]) to getfqdn(name=''), but not sure if it helps with improving the wording much.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think that both changing the wording and changing the signature are worth to do.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Done, let me know if there's anything that looks off. For reference, here's the preview of getfqdn.

Comment thread Doc/library/socket.rst
*backlog* is the queue size passed to :meth:`socket.listen`; if not specified
, a default reasonable value is chosen.
*backlog* is the queue size passed to :meth:`socket.listen`; if not
specified, a default reasonable value is chosen.

slateny Mar 3, 2024
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Small formatting fix to remove space before comma for this:

Copy link
Copy Markdown

The following commit authors need to sign the Contributor License Agreement:

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review docs Documentation in the Doc dir skip news stale Stale PR or inactive for long period of time.

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL