8000 gh-99242 Catch OSError when calling getloadavg in libregrtest by kgdiem · Pull Request #106705 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-99242 Catch OSError when calling getloadavg in libregrtest #106705

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

gh-99242 Catch OSError when calling getloadavg in libregrtest #106705

wants to merge 4 commits into from

Conversation

kgdiem
Copy link
Contributor
@kgdiem kgdiem commented Jul 13, 2023 •
edited by bedevere-bot
Loading

Per suggestions in the comments for #99242 I added a try/except around the call to os.getloadavg.

Perhaps a logged warning here is appropriate as well?

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@bedevere-bot bedevere-bot mentioned this pull request Jul 13, 2023
Closed
@ghost
Copy link
ghost commented Jul 13, 2023 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@kgdiem kgdiem requested a review from CharlieZhao95 July 13, 2023 09:48
StyXman
StyXman reviewed Jul 19, 2023
View reviewed changes
Lib/test/libregrtest/main.py
return os.getloadavg()[0]
try:
return os.getloadavg()[0]
except OSError:
Copy link

Choose a reason for hiding this comment

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

The raised exception does not have any other information that .args == ('Load averages are unobtainable',), so it's ok if we don't inspect it more to make sure we're catching only the issue at hand.

StyXman
StyXman approved these changes Jul 19, 2023
View reviewed changes
Copy link
@StyXman StyXman left a comment

Choose a reason for hiding this comment

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

Tested on a CentOS 7.9 chroot without /proc mounted. On newer OSs /proc is not used but a syscall sysinfo().

@basbloemsaat basbloemsaat mentioned this pull request Jul 13, 2024
Merged
@encukou
Copy link
Member
encukou commented Jul 13, 2024

Merged in #121663. Thank you for the fix!

@encukou encukou closed this Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StyXman StyXman StyXman approved these changes

@rshah713 rshah713 rshah713 approved these changes

@CharlieZhao95 CharlieZhao95 Awaiting requested review from CharlieZhao95

Assignees
No one assigned
Labels
awaiting core review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kgdiem @bedevere-bot @encukou @StyXman @rshah713 @CharlieZhao95
0