Skip to content

bpo-38480: resource.setrlimit() should raise PermissionError #16804

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

Closed
wants to merge 4 commits into from
Closed

bpo-38480: resource.setrlimit() should raise PermissionError #16804

wants to merge 4 commits into from

Conversation

giampaolo
Copy link
Contributor

@giampaolo giampaolo commented Oct 15, 2019

Copy link
Member

@zhangyangyu zhangyangyu left a comment

Choose a reason for hiding this comment

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

The whatsnew and changelog are wrong. It's ValueError and setrlimit. And the document of setrlimit should also be updated. Currently it says: raises ValueError if a process tries to raise its hard limit. I think we need a versionchanged in 3.9 documents when a not super user do this PermissionError is raised.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@csabella
Copy link
Contributor

@giampaolo, please take a look at the code review. Thanks!

@giampaolo
Copy link
Contributor Author

@zhangyangyu could you please take one last look?

@zhangyangyu zhangyangyu changed the title bpo-38480: resource.setrusage() should raise PermissionError bpo-38480: resource.setrlimit() should raise PermissionError Feb 6, 2020
@@ -0,0 +1,2 @@
resource.setrusage() now raises PermissionError instead of KeyError if the
Copy link
Member

Choose a reason for hiding this comment

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

it's setrlimit

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@giampaolo giampaolo closed this by deleting the head repository Aug 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants