Skip to content

_socket.py: fix for systems where AI_NUMERICSERV is not defined #3135

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
Mar 31, 2025

Conversation

barracuda156
Copy link

Rationale: on some platforms AI_NUMERICSERV may not be defined. In particular, old versions of macOS, possibly some other. While affected users are probably few, it is a trivial fix, which makes trio usable (for example, yewtube app works then).

Credits to @CoolCat467
Closes: #3133

Copy link

codecov bot commented Nov 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.17606%. Comparing base (67f7455) to head (a821d36).
Report is 118 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main       #3135         +/-   ##
===================================================
+ Coverage   56.17373%   56.17606%   +0.00233%     
===================================================
  Files            246         246                 
  Lines          37530       37532          +2     
  Branches        2538        2538                 
===================================================
+ Hits           21082       21084          +2     
  Misses         16320       16320                 
  Partials         128         128                 
Files with missing lines Coverage Δ
src/trio/_socket.py 100.00000% <100.00000%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@CoolCat467 CoolCat467 left a comment

Choose a reason for hiding this comment

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

Looks good, but needs newsfragment (that is why continuous integration is failing). newsfragments folder's README file has more information.

@barracuda156
Copy link
Author

@CoolCat467 That one CI which fails is unrelated, right?

@A5rocks
Copy link
Contributor

A5rocks commented Nov 12, 2024

@CoolCat467 That one CI which fails is unrelated, right?

Docs are failing because your newsfragment is using markdown not rST. Try doing socket.AI_NUMERICSERV instead of AI_NUMERICSERV (for the automatic linking) or doubling the backticks.

It's a bit confusing because the logs show two warnings, but the one you should care about is:

/home/docs/checkouts/readthedocs.org/user_builds/trio/checkouts/3135/docs/source/history.rst:42: WARNING: py:obj reference target not found: AI_NUMERICSERV [ref.obj]

I don't think the other one makes it fail, though I may be wrong.

Comment on lines 172 to 170
# AI_NUMERICSERV may be missing on some older platforms, so use it when available.
_NUMERIC_ONLY = _stdlib_socket.AI_NUMERICHOST
_NUMERIC_ONLY |= getattr(_stdlib_socket, "AI_NUMERICSERV", 0)
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to be a bit more specific here, otherwise future people that stumble on this comment will have to do a bunch of research to figure out why/when it might be missing. And as I said in the issue, perhaps even a note on why it's needed despite being deprecated since a decade+

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just linking the issue will work? That has info.

Copy link
Contributor

@A5rocks A5rocks Dec 7, 2024

Choose a reason for hiding this comment

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

@barracuda156 could you fix this? I think this is the only problem before merging.

(well, barring rejecting this whole change on basis of it being something we cannot test for a system we could not support. IMO this change is fine but @jakkdl didn't respond to the response you left on the issue.)

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, this got missed.
I will return to this tomorrow.

Copy link
Author

Choose a reason for hiding this comment

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

Added a link now to the issue into the comment.

@A5rocks
Copy link
Contributor

A5rocks commented Mar 20, 2025

Bump @barracuda156

Copy link
Member

@CoolCat467 CoolCat467 left a comment

Choose a reason for hiding this comment

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

With newsfragment and link to discussion, I think this should be good to go.

@A5rocks A5rocks requested a review from jakkdl March 22, 2025 06:40
@jakkdl jakkdl enabled auto-merge March 31, 2025 08:50
@jakkdl jakkdl added this pull request to the merge queue Mar 31, 2025
Merged via the queue into python-trio:main with commit d6c4cae Mar 31, 2025
42 of 43 checks passed
@trio-bot
Copy link

trio-bot bot commented Mar 31, 2025

Hey @barracuda156, it looks like that was the first time we merged one of your PRs! Thanks so much! 🎉 🎂

If you want to keep contributing, we'd love to have you. So, I just sent you an invitation to join the python-trio organization on Github! If you accept, then here's what will happen:

  • Github will automatically subscribe you to notifications on all our repositories. (But you can unsubscribe again if you don't want the spam.)

  • You'll be able to help us manage issues (add labels, close them, etc.)

  • You'll be able to review and merge other people's pull requests

  • You'll get a [member] badge next to your name when participating in the Trio repos, and you'll have the option of adding your name to our member's page and putting our icon on your Github profile (details)

If you want to read more, here's the relevant section in our contributing guide.

Alternatively, you're free to decline or ignore the invitation. You'll still be able to contribute as much or as little as you like, and I won't hassle you about joining again. But if you ever change your mind, just let us know and we'll send another invitation. We'd love to have you, but more importantly we want you to do whatever's best for you.

If you have any questions, well... I am just a humble Python script, so I probably can't help. But please do post a comment here, or in our chat, or on our forum, whatever's easiest, and someone will help you out!

@barracuda156 barracuda156 deleted the AI_NUMERICSERV branch March 31, 2025 10:08
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.

AttributeError: module 'socket' has no attribute 'AI_NUMERICSERV'. Did you mean: 'NI_NUMERICSERV'?
4 participants