8000 gh-118986: expose socket.IPV6_RECVERR by chrysn · Pull Request #118987 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-118986: expose socket.IPV6_RECVERR #118987

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

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

chrysn
Copy link
Contributor
@chrysn chrysn commented May 13, 2024

This adds two socket option numbers, without which the existing MSG_ERRQUEUE gives barely any benefits.

@chrysn chrysn marked this pull request as ready for review May 13, 2024 08:32
@chrysn chrysn changed the title gh-118986: expose socket.IPV6_RECVERR and socket.IP_RECVERR gh-118986: expose socket.IPV6_RECVERR Oct 16, 2024
@chrysn
Copy link
Contributor Author
chrysn commented Oct 16, 2024

This has missed the 3.14 alpha; may I ask for a review again to have this available next round?

@Eclips4 and @jeremyhylton would be suitable reviewers given the history of #120056.

@Eclips4 Eclips4 self-requested a review October 16, 2024 10:27
@Eclips4
Copy link
Member
Eclips4 commented Oct 17, 2024

Please, modify the lines stated below to include the IPV6_RECVERR.

.. versionchanged:: 3.14
Added missing ``IP_RECVERR``, ``IP_RECVTTL``, and ``IP_RECVORIGDSTADDR``
on Linux.

@chrysn
Copy link
Contributor Author
chrysn commented Oct 17, 2024

Adjusted as requested.

You pointed at the lines for Python 3.14 -- do I read this right that this PR can become part of 3.14 still?

@Eclips4
Copy link
Member
Eclips4 commented Oct 17, 2024

You pointed at the lines for Python 3.14 -- do I read this right that this PR can become part of 3.14 still?

Yes, you understand that correctly! Python 3.14 is in development stage and two days ago was released the first alpha of 3.14, so we have more than half a year to implement new features for 3.14 version. For more information see https://peps.python.org/pep-0745/

If your question was about backporting to versions 3.12 and 3.13, sorry, we can't do that, because this PR is considered a feature request and versions 3.12 and 3.13 only accept security and bug fixes.

Copy link
Member
@Eclips4 Eclips4 left a comment

Choose a reason for hiding this comment

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

Thanks! I'll let it unmerged for some time to get a review from others if they have something to say.

Copy link
Member
@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks!

@Eclips4
Copy link
Member
Eclips4 commented Oct 17, 2024

@chrysn, in the future, please avoid force pushes as they make PRs difficult to review. Commits are squashed anyway before merging, so it doesn't matter how many commits you need to complete a PR :)

@Eclips4
Copy link
Member
Eclips4 commented Oct 17, 2024

This adds two socket option numbers, without which the existing MSG_ERRQUEUE gives barely any benefits.

You've said that there are two new socket constants, but in fact, this PR contains only one. Did you forget to add the second one?

@chrysn
Copy link
Contributor Author
chrysn commented Oct 17, 2024

The issue was originally about adding two constants (I've updated the title but kept the original post).

One of the constants was already added after this PR was filed in a different PR (#120056), so when I updated this PR, only one was left.

@sobolevn sobolevn merged commit b454662 into python:main Oct 17, 2024
37 checks passed
@sobolevn
Copy link
Member

Thank you!

@chrysn chrysn deleted the ip_recverr branch October 17, 2024 17:59
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0