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.

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

Drop root privileges for celery worker #3272

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
+4 −2
Open

Drop root privileges for celery worker #3272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,7 @@ integreat_cms/xliff/download
.postgres

# Celery
celerybeat-schedule.db
celerybeat-schedule
celerybeat-schedule.db
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
celerybeat-schedule.db
celerybeat-schedule.dat

Copy link
Contributor

Choose a reason for hiding this comment

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

celerybeat-schedule.db is apparently also a thing 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I'm wondering where the .dat file is coming from. I never had that :D

celerybeat-schedule.bak
celerybeat-schedule.dir
2 changes: 1 addition & 1 deletion tools/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ done
listen_for_devserver &

# Run Celery worker process
celery -A integreat_cms.integreat_celery worker -l INFO -B --concurrency=1 &
deescalate_privileges celery -A integreat_cms.integreat_celery worker -l INFO -B --concurrency=1 &
Copy link
Contributor

Choose a reason for hiding this comment

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

This is more a question than a comment, but: are we dependent on celery for the dev env? Because as far as I can tell, I can still do everything I normally would in the CMS when commenting out this line. I get that background workers won't be available, but will this crash the dev server, or just lead to errors in the logs?

If it is "optional", I would love the option to set an environment variable to skip starting celery (and thus not need redis installed).

If it is mandatory, I think the docs should mention that redis is also a mandatory prerequisite to running the dev server.

Copy link
Member

Choose a reason for hiding this comment

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

I'd also be in favor of not doing this by default in debug mode, because it can really get annoying ^^
And sometimes its just necessary to not have redis running (For example to debug some db calls)

Copy link
Member Author

@svenseeberg svenseeberg Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I have no strong opinion here. But I'd recommend to move all cron tasks to Celery scheduled tasks. Then the application can manage tasks in source code instead of cron configurations on the server. In the end Celery will simply not work if Redis is not running.


# Start Integreat CMS development webserver
deescalate_privileges integreat-cms-cli runserver "localhost:${INTEGREAT_CMS_PORT}"
Loading