added time limit to celery tasks #743
Conversation
Signed-off-by: swastik959 <Sswastik959@gmail.com>
kairoaraujo
left a comment
kairoaraujo
left a comment
There was a problem hiding this comment.
Hi @swastik959,
Thank you for your contribution! 🎉
I left a comment for us to elaborate more this PR
…ded them on task level Signed-off-by: swastik959 <Sswastik959@gmail.com>
|
@kairoaraujo I have done the relevant changes as suggested by you please review them |
kairoaraujo
changed the title
kairoaraujo
left a comment
kairoaraujo
left a comment
There was a problem hiding this comment.
I reviewed it and it looks good for repository_service_tuf_worker main tasks.
I just miss the UT for the app.py (see tests/unit/test_app.py)
Please, see my commends about the other tasks
Great work @swastik959
|
|
||
|
|
||
| @app.task(serializer="json", queue="rstuf_internals") | ||
| @app.task(serializer="json", queue="rstuf_internals" , task_time_limit=TASK_TIME_LIMIT , task_soft_time_limit=TASK_SOFT_TIME_LIMIT) |
There was a problem hiding this comment.
I'm not sure if we should put the task_[soft]_limit for the bump_online_role right now, for some reasons:
- It will conflict directly with the
BOR_LOCKwe have to handle - Depending on the number of delegated online roles, it will be a problem
- Using the same task limit time to the add/remove artifacts tasks
I'm in favor here of filing a new specific issue for adding task time limit for bump_online_roles
| return None | ||
|
|
||
| @app.task(serializer="json", queue="rstuf_internals") | ||
| @app.task(serializer="json", queue="rstuf_internals", task_time_limit=TASK_TIME_LIMIT , task_soft_time_limit=TASK_SOFT_TIME_LIMIT) |
There was a problem hiding this comment.
(same as _update_online_role)
|
|
||
|
|
||
| @app.task(serializer="json", queue="rstuf_internals") | ||
| @app.task(serializer="json", queue="rstuf_internals", task_time_limit=TASK_TIME_LIMIT , task_soft_time_limit=TASK_SOFT_TIME_LIMIT) |
There was a problem hiding this comment.
this task is a chain task from bump_online_roles.
I think we should use the same config here, see my comment on bump_online_roles -- I think it should part of a specific change from there.
|
@kairoaraujo should I just undo the changes in other functions apart from main one ? |
2 participants
added timr limit to celery tasks in app.py for around an hour
solves #708