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

Config: deprecated notification for projects without config file #10354

Merged
merged 33 commits into from Jun 6, 2023

Conversation

humitos
Copy link
Member

@humitos humitos commented May 25, 2023

When we detect a build project is built without a Read the Docs configuration file (.readthedocs.yaml) we show multiple notifications:

  • a static warning message in the build detail's page
  • a persistent on-site notification to all maintainers/admin of the project
  • send a weekly email (at most)

This is the initial step to attempt making users to migrate to our config file v2, giving them a enough window to do this and avoid breaking their builds in the future.

Note: I didn't put too much effort on the copy of the notifications. I'd appreciate some input here, @agjohnson.


Static warning message

Screenshot_2023-05-25_13-52-02


On-site notification

Screenshot_2023-05-25_13-51-24


Email

I don't have a rendered version because I'm using the dummy email backend, but it works fine.

Closes #10348

When we detect a build is built without a Read the Docs configuration
file (`.readthedocs.yaml`) we show multiple notifications:

- a static warning message in the build detail's page
- a persistent on-site notification to all maintainers/admin of the project
- send a weekly email (at most)

This is the initial step to attempt making users to migrate to our config file
v2, giving them a enough window to do this and avoid breaking their builds in
the future.

Closes #10348
@humitos humitos requested a review from agjohnson May 25, 2023 11:55
@humitos humitos requested review from a team as code owners May 25, 2023 11:55
@humitos humitos requested a review from stsewd May 25, 2023 11:55
Instead of sending an onsite notification on each build,
we use a scheduled Celery task that runs once a week to send them.

It filter projects that are not using the v2 config file.
@humitos
Copy link
Member Author

humitos commented May 29, 2023

@agjohnson I updated the code here with all the suggestions.

Note that the scheduled Celery task contains a db-intense query. Well, it's not just one intense CPU query, but they are multiple small ones that may affect the db. I'd suggest to take one web-celery instance, update the code with this PR, run a specific queue (--queue="config-file-notifications") and trigger the task on it before deploying it to production, so we can see how it behaves. I can do this manually tomorrow, so we send the notifications when they are scheduled to be sent 😉

@humitos humitos requested a review from agjohnson May 29, 2023 10:13
@humitos humitos changed the title Config: deprecated notification for builds without config file Config: deprecated notification for projects without config file May 29, 2023
@humitos
Copy link
Member Author

humitos commented May 29, 2023

I did a quick test of that chunk of code in production using 35k projects as limit and it took 205s, which increasing the CPU load of the db in a noticeable way. After that, I tested it with 55k projects and it took 295s without noticeable MAX CPU increase in the graphs. I think we will be fine.

Note: we have around ~550k projects

@humitos
Copy link
Member Author

humitos commented May 30, 2023

If we are happy with this blog post readthedocs/blog#219, we can merge it and change all the links that point to the config file v2 documentation to point to this blog post that explains in a simpler way how to update/create the config file.

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

Just some small tweaks, nothing that needs to block this. I think we'll be tweaking the task query a little bit going forward, but for now it might just incur some false negatives if a project has a lot of active versions. But this will only be annoying to the maintainer, and would still accomplish notifying users that we need to update their configuration.

readthedocs/projects/tasks/utils.py Outdated Show resolved Hide resolved
readthedocs/projects/tasks/utils.py Outdated Show resolved Hide resolved
readthedocs/projects/tasks/utils.py Outdated Show resolved Hide resolved
readthedocs/projects/tasks/utils.py Outdated Show resolved Hide resolved
@humitos
Copy link
Member Author

humitos commented May 31, 2023

Note that the scheduled Celery task contains a db-intense query. Well, it's not just one intense CPU query, but they are multiple small ones that may affect the db.

The query seems to not be a problem for our database. I just executed it in production and checked the CPU usage and it's fine. It's around the normal in EU mornings, so we are fine I guess.

However, it takes forever to calculate the initial list of projects we need to contact. I think we can reduce this by removing the spam projects from this query. This will also help us to reduce the number of emails we send and avoid killing our email reputation too. I will add this now and continue with the tests here.

Also, add log for current project in case we need to recover from that task.
@humitos
Copy link
Member Author

humitos commented May 31, 2023

OK. The query ran and finished on production:

In [38]: deprecated_config_file_used_notification()
Projects: 82872
Seconds: 1741

This is 30m to get all the projects that we need to contact. Using spam_score = 300 this gave us 82k (from 250k) projects to contact.

The notification will include all the projects the user is admin that are
affected by this deprecation. Users will receive at most one notification per
week.
@humitos
Copy link
Member Author

humitos commented Jun 1, 2023

This is getting closer.

Tune query to be one notification per user, instead of one per user per project

This is done, but I'd like to have more eyes here since it was not trivial. I think it may be possible to make it more db performant, but that will complicate the query making it more appealing for mistakes.

I did a test running these queries in production and I found that we have ~82k projects affected by this and ~68k users to contact. Some of these users have multiple projects affected. I estimate the task requires around 1h30m to complete with this amount of users based on my tests.

Include cutoff date in the notification message

I added them as a starting idea. There are more work to do here, tho, but we need to finish the blog post first.

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

Just a couple of notes before I forget

readthedocs/projects/tasks/utils.py Outdated Show resolved Hide resolved
readthedocs/projects/tasks/utils.py Outdated Show resolved Hide resolved
@humitos
Copy link
Member Author

humitos commented Jun 6, 2023

OK. I took some decisions here and updated this code:

  • we are sending onsite notifications to about 60k users
  • we are not sending emails just yet because we want to test this a little more before sending massive emails (it's commented for now)
  • we are deploying the code for emails notifications so we can send some emails from production and test everything is in place
  • next week's deploy will include email notifications to be sent weekly
  • we can send the emails notification manually if we feel comfortable just by running the task from the django shell
  • if we need to make some last-minute adjustment, we can hotfix since this code is not going to be auto-executed until tomorrow

Let me know if there is anything you want to change here and I will do it. Hopefully, before this code is executed tomorrow.

@humitos humitos merged commit e3b5484 into main Jun 6, 2023
2 of 5 checks passed
@humitos humitos deleted the humitos/build-without-config-deprecation branch June 6, 2023 07:33
humitos added a commit that referenced this pull request Aug 1, 2023
Define a weekly task to communicate our users about the deprecation of
`build.image` using the deprecation plan we used for the configuration file v2
as well.

- 3 brownout days
- final removal date on October 2nd
- weekly onsite/email notification on Wednesday at 11:15 CEST (around 3.5k projects affected)
- allow to opt-out from these emails
- feature flag for brownout days
- build detail's page notification

Related:
* readthedocs/meta#48
* #10354
* #10587
humitos added a commit that referenced this pull request Aug 3, 2023
Define a weekly task to communicate our users about the deprecation of
`build.image` using the deprecation plan we used for the configuration file v2
as well.

- 3 brownout days
- final removal date on October 2nd
- weekly onsite/email notification on Wednesday at 11:15 CEST (around ~22k projects affected)
- allow to opt-out from these emails
- feature flag for brownout days
- build detail's page notification

Related:
* readthedocs/meta#48
* #10354
* #10587
humitos added a commit that referenced this pull request Aug 9, 2023
…10589)

* Deprecation: notification and feature flag for `build.image` config

Define a weekly task to communicate our users about the deprecation of
`build.image` using the deprecation plan we used for the configuration file v2
as well.

- 3 brownout days
- final removal date on October 2nd
- weekly onsite/email notification on Wednesday at 11:15 CEST (around 3.5k projects affected)
- allow to opt-out from these emails
- feature flag for brownout days
- build detail's page notification

Related:
* readthedocs/meta#48
* #10354
* #10587

* Deprecation: notification and feature flag for `build.image` config

Define a weekly task to communicate our users about the deprecation of
`build.image` using the deprecation plan we used for the configuration file v2
as well.

- 3 brownout days
- final removal date on October 2nd
- weekly onsite/email notification on Wednesday at 11:15 CEST (around ~22k projects affected)
- allow to opt-out from these emails
- feature flag for brownout days
- build detail's page notification

Related:
* readthedocs/meta#48
* #10354
* #10587

* Review and update logic

* Start emailing people with projects building from 3 years ago

* Apply suggestions from code review

Co-authored-by: Anthony <aj@ohess.org>
Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>

* Add HTML version of the email

* Codify brownout dates and remove the feature flag

Follows the suggestion from https://github.com/readthedocs/blog/pull/233/files#r1283479184

* Use UTC datetimes to compare

* Contact projects with a build in the last 3 years

We will start with 3 years timeframe first and then lower it down to 1 year.

---------

Co-authored-by: Anthony <aj@ohess.org>
Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>
humitos added a commit that referenced this pull request Aug 9, 2023
* Deprecation: notification and feature flag for `build.image` config

Define a weekly task to communicate our users about the deprecation of
`build.image` using the deprecation plan we used for the configuration file v2
as well.

- 3 brownout days
- final removal date on October 2nd
- weekly onsite/email notification on Wednesday at 11:15 CEST (around 3.5k projects affected)
- allow to opt-out from these emails
- feature flag for brownout days
- build detail's page notification

Related:
* readthedocs/meta#48
* #10354
* #10587

* Deprecation: notification and feature flag for `build.image` config

Define a weekly task to communicate our users about the deprecation of
`build.image` using the deprecation plan we used for the configuration file v2
as well.

- 3 brownout days
- final removal date on October 2nd
- weekly onsite/email notification on Wednesday at 11:15 CEST (around ~22k projects affected)
- allow to opt-out from these emails
- feature flag for brownout days
- build detail's page notification

Related:
* readthedocs/meta#48
* #10354
* #10587

* Review and update logic

* Start emailing people with projects building from 3 years ago

* Apply suggestions from code review

Co-authored-by: Anthony <aj@ohess.org>
Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>

* Add HTML version of the email

* Codify brownout dates and remove the feature flag

Follows the suggestion from https://github.com/readthedocs/blog/pull/233/files#r1283479184

* Use UTC datetimes to compare

* Deprecation: codify browndates for "no config file deprecation"

Follows https://github.com/readthedocs/blog/pull/233/files#r1283479184

This way, we don't have to enable anything manually.
These dates are already communicated to users and will be automatically applied.

---------

Co-authored-by: Anthony <aj@ohess.org>
Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>
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.

Deprecation: add warning for required configuration
4 participants