Skip to content
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

Update faq.rst and fix technical mistake on ulimit #2173

Merged
merged 6 commits into from
May 7, 2023
Merged

Update faq.rst and fix technical mistake on ulimit #2173

merged 6 commits into from
May 7, 2023

Conversation

julyclyde
Copy link
Contributor

No description provided.

@tilgovi
Copy link
Collaborator

tilgovi commented Nov 14, 2019

Nice! I'm not sure the "using sudo" sentence is necessary. The recommendation is very clear! It might also be good to add a note about the --user and --group options for dropping privileges.

Should we try to recommend any reading about /etc/security/limits.conf or systemd service limit overrides?

@julyclyde
Copy link
Contributor Author

@tilgovi Yes you are right, modifying limits.conf or using systemd unit file are also correct way besides.

I don't want to talk about external dependencies just now, just want to focus on the problem itself, so I gave my word in this PR. Maybe other words are better. Sorry for my poor English.

Copy link
Owner

@benoitc benoitc left a comment

Choose a reason for hiding this comment

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

ulimit would definitely work for a terminal and some users so I would make the line added as a warning note to highlight it.

@julyclyde
Copy link
Contributor Author

No, it should not work.
ulimit is a built-in command of shell, it take effect only in the shell and its children processes.
sudo ulimit would run setrlimit() in a sudo process, and then return.

@julyclyde
Copy link
Contributor Author

You may check it in /proc//limits

@benoitc
Copy link
Owner

benoitc commented Nov 18, 2019 via email

@julyclyde
Copy link
Contributor Author

OK, I'll change the PR later. I need some time to consider how to express this.

@benoitc
Copy link
Owner

benoitc commented Nov 18, 2019 via email

docs/source/faq.rst Show resolved Hide resolved
Copy link
Owner

@benoitc benoitc left a comment

Choose a reason for hiding this comment

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

the .. warning:: must be related to the paragraph, see my comment inline.

@benoitc benoitc merged commit 8a41cd2 into benoitc:master May 7, 2023
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.

None yet

3 participants