-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add management command to run all necessary tasks after a release. #1562
Add management command to run all necessary tasks after a release. #1562
Conversation
brianjp93
commented
Dec 19, 2024
•
edited
Loading
edited
- fixes Add periodic tasks for updating release report data #1519
- add button to version admin to execute task
- use @ locked decorator to ensure only one instance of the task is running at any time.
- If a user_id is given to the command, send that user email notifications when it starts and ends.
During a release, the script Now, two buttons and two REST endpoints... To consider - should these new steps be automatically triggered at the end of the new_versions import process. Or add a third button and endpoint which will consolidate all the tasks. |
How long is this taking locally? I wonder if using Celery's Canvas to create an actual task chain would be wiser. Maybe a future optimization if this is performing reasonably. |
@sdarwin I wasn't aware of that script. I think it makes sense if that script hits a new endpoint which consolidates required tasks post-release. This PR already makes an endpoint available at /admin/versions/version/post_release_tasks/. Perhaps we can hit that instead? |
@GregKaleka Locally, it took 14 minutes but definitely has the potential to take longer. I think that the piece with the worst potential is the fetching of slack messages, but they are also being fetched intermittently so it may not be so bad. Would a task chain be helpful? Maybe some pieces could be run in parallel but that would make tracking the overall task's progression more difficult. |
bdd4b3c
to
10191e4
Compare
- fixes boostorg#1519 - add button to version admin to execute task - use @locked decorator to ensure only one instance of the task is running at any time. - If a user_id is given to the command, send that user email notifications when it starts and ends.
10191e4
to
56a46ea
Compare
@brianjp93 , what are the dependencies between these? what is the timing between them /admin/versions/version/new_versions/ Just exploring the issue. some ideas. not certain.
For example, what if the endpoint |
@sdarwin This new task is meant to be an all encompassing group of tasks to be run at release time.
|
Ha! Then nevermind! I saw the name "post_release_tasks", and the description "after a release", and assumed these were later tasks AFTER the release was already done. (slack reports, mailing list statistics, etc). In fact, The release is not live until the website displays it. So the script endpoint will be the It would not be so far-fetched to name this |
Ah I see, my wording is wrong then. Sorry for the confusion, I will update the naming. |
No problem, you can make a case for both names, they are both reasonable, in a way. |
Before,
And now? Still and What happens if you run Edit: I see you wrote "meant to be idempotent". So, nothing new will happen, I guess. |
@sdarwin If these tasks are necessary for the beta as well as full releases, it may make sense to use Maybe @rbbeeston has a better idea about if this new task should run after beta releases as well as full releases? |
We won't be running reports on beta releases. Typically we would run a draft report, check for issues, and rerun until there is one ready to make public. So I don't see a clear need to do anything special for them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm goos with this when Greg approves
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! One very minor cleanup suggestion, but approved.