- 🇺🇸United States smustgrave
Brought this up in #yes-no-queue and @catch mentioned this could still be valid.
Sounds like the request is to be able to select a day when selecting weekly under "Check for Updates"
- Assigned to yash.rode
- Status changed to Needs work
12 months ago 8:57am 21 September 2023 - Open on Drupal.org →Environment: PHP 8.2 & MySQL 8last update
12 months ago Not currently mergeable. - @yashrode opened merge request.
- Open on Drupal.org →Environment: PHP 8.2 & MySQL 8last update
12 months ago Not currently mergeable. - @yashrode opened merge request.
- last update
12 months ago Custom Commands Failed - last update
12 months ago 30,168 pass - last update
12 months ago 30,168 pass - Issue was unassigned.
- Status changed to Needs review
12 months ago 11:04am 22 September 2023 - 🇮🇳India yash.rode pune
Added config and changed update_cron hook to take day of week in to account.
- Status changed to Needs work
12 months ago 3:33pm 22 September 2023 - 🇺🇸United States smustgrave
With the schema updates image we will need an upgrade path for that.
Also will need test coverage
- last update
12 months ago 30,209 pass - Status changed to Needs review
12 months ago 10:31am 25 September 2023 - 🇮🇳India yash.rode pune
Can someone, please, provide instructions on how can we write a test for this?
- 🇺🇸United States smustgrave
Brought could use a mock right? To pretend it's Thursday.
- last update
12 months ago 30,364 pass, 2 fail - last update
12 months ago 30,362 pass - Status changed to Needs work
12 months ago 5:35pm 27 September 2023 - 🇺🇸United States smustgrave
Just realized Issue summary has some parts marked TBD that will need to be filled in.
But the test, least from what I can tell, is testing sending the notification always on "today" so not testing the Thursday feature I imagine.
- last update
12 months ago 30,366 pass - Status changed to Needs review
12 months ago 8:48am 28 September 2023 - 🇮🇳India yash.rode pune
But the test, is testing sending the notification always on "today" so not testing the Thursday feature I imagine.
The default day is Thursday, but we can set it to anything we want. In test, I am setting the update day to today and checking if I get the update notification email today. I couldn't find a way to pretend the other day to run cron. Other than that, I have added one more test case to confirm this by checking the notification is not sent on other days than the configured day.
- Status changed to RTBC
12 months ago 7:43pm 28 September 2023 - Status changed to Needs review
11 months ago 1:55pm 29 September 2023 - 🇺🇸United States xjm
This is a great idea!
Have we accounted for the fact that the week starts on different days depending on your locale? "Thursday" can be counted as either day 4 or day 5, depending on whether the location's week starts on a Sunday. Furthermore, while scheduling updates for the day after the default security window is good -- but! It doesn't help Australia and New Zealand, because Thursday for them starts before the scheduled hours of the security window. So they need it the most but they'll always be one full week out of date.
I think we're on the right track here, but we need to think a little more about the datetime aspects, so that we have better defaults and configuration options that get localized properly. I'd suggest reaching out to DateTime maintainers as well as people who are actually from Australia and New Zealand, to get a sense of an internationalized version of this that's less US/EU-centric in its conception.
@yash.rode, FWIW, your persepctive on how this affects Indian sites is probably worthwhile as well, since midnight actually happens smack in the middle of the security release window for India (if you are still in Pune that is).
Tagging for subsystem maintainer review, but I actually mean the DateTime maintainers who understand the dark corners of timezone compatibility. There is no tag for "Needs APAC sanity check." 😉
- Status changed to Needs work
11 months ago 8:21pm 2 October 2023 - 🇬🇧United Kingdom joachim
> Since SAs go out on Wednesdays this means it could be a full week between the SA and the notification.
I can't actually find that information online. Googling "drupal SA window" gets me only past SAs. https://www.drupal.org/security → doesn't say.
- last update
11 months ago 30,374 pass - last update
11 months ago 30,374 pass - last update
11 months ago 30,395 pass - Status changed to Needs review
11 months ago 12:27pm 11 October 2023 - 🇮🇳India yash.rode pune
Changed the description to take into account timezone and changed the day format.
- Status changed to RTBC
11 months ago 7:26pm 18 October 2023 - 🇺🇸United States smustgrave
Still needs submaintainer sign off but to keep the issue from stalling moving to RTBC.
Seems all threads have been resolved
Verified post_update still runs without issue. - last update
11 months ago 30,420 pass - Status changed to Needs work
11 months ago 11:55pm 18 October 2023 - 🇺🇸United States dww
Thanks for keeping this moving forward! I opened some new threads on the MR. The most significant being this:
Nice touch on this help text to localize it for the person configuring the site. However, the information about 6pm UTC is potentially frustrating since people have no control over what time of day the check is happening. 😞 That's nice to know it'll be "Thursday at 2am" for me, but does that mean I have to set it for Friday to be "safe", since maybe this will run at 12:01am on Thursday?
I'm not sure what to do with this. I think it's getting too complicated to add a time of day config knob, too, but maybe we "need" that. Or we need to invent something arbitrary like we always do these checks as close to "noon local time" as possible.
But if we're going to go that far, why make this a new config knob at all? Why not just automatically have the check logic know that if you're only checking weekly, it should know this:
> Security releases happen between 16:00 UTC and 22:00 UTC.
If the knob is configured for weekly checks, it should always aim to check as soon after Wednesday at 22:00 UTC as possible./strong>
In theory, that policy could change, and we'll regret hard-coding this, but we release core often enough that if we ever did need to change this, we could...
I have some regret I didn't consider this when I first reviewed the issue, but at least I thought of it now! 😉 Thankfully, it should result in a much smaller change to core overall, and we don't have to further complicate the UI at all.
If there are no objections to my counter-proposal, this needs a summary update and an almost entirely different change. Perhaps it'd be cleaner in a new MR.
Thanks/sorry!
-Derekp.s. Saving credit for everyone who's participated so far, since everything has been a meaningful contribution towards fixing this.
- 🇺🇸United States tedbow Ithaca, NY, USA
@dww #34 Sounds like a good idea to me.
This probably not an issue but just thought I would raise it just in case....
Is the Update XML served by a CDN? If not would there be a problem with every site's cron requesting at around the same time? I figure it will not be a problem because cron is probably not going to run exactly on the hour for sites so it will vary.Again i am in favor of @dww's idea just thought I would raise that issue in case it raise any flags with anyone more knowledgeable about such things than myself
- 🇬🇧United Kingdom catch
One thing with #36, is it will only happen on sites that update to the release with this in so it should be a small problem that gradually gets worse rather than immediate, giving time tweaknedge caches if needed. Might also result in more cache hits if we're lucky too.
- 🇺🇸United States drumm NY, US
Is the Update XML served by a CDN? If not would there be a problem with every site's cron requesting at around the same time? I figure it will not be a problem because cron is probably not going to run exactly on the hour for sites so it will vary.
As long as this is true, cron doesn’t run exactly on the hour, it’s all good. Our CDN does handle spikes at the top of the hour already, since it is common configuration. Its better to move away from that for any default configuration.
- 🇩🇰Denmark ressa Copenhagen
Updates and alert emails are sent once every 24 hours, which can result in an update alert getting sent 23 hours after a security update was released.
Since Drupal security updates are usually sent out around 18:00 GMT, it would be nice if there was a setting to define exactly when to check for updates and send email updates, for example at 20:00 GMT.
Maybe it's even worth considering a new option, which is "Check for updates > Daily > Every Hour" if you as an administrator want to make sure you always get email alerts ASAP?
- 🇬🇧United Kingdom catch
Maybe it's even worth considering a new option, which is "Check for updates > Daily > Every Hour" if you as an administrator want to make sure you always get email alerts ASAP?
That would be a lot of additional traffic given the relatively low frequency of security releases on Drupal.org, update module already makes one request per installed project iirc, so with 200 projects installed that's 200 requests per week, but if we did it per hour it would be 33,600 requests per week.
If we wanted to do something like that, I think we'd need to implement some kind of security release canary - e.g. an endpoint on Drupal.org that's a timestamp of the last security release (against any project), and if it's newer than when we last checked for updates, trigger a check then. Having written this out, that might be a viable approach, then we could remove the setting altogether and check every cron run.
- 🇺🇸United States dww
I still think #34 makes more sense and would be vastly easier to implement than #39 or #40
- 🇺🇸United States dww
P.s. if you want “immediate” email notifications of security updates, subscribe to the security announcements newsletter and/or consume the feed via RSS.
- 🇺🇸United States drumm NY, US
Checking frequently isn’t an issue for traffic server-side, the CDN absorbs that.
In highly critical security releases, even checking at the end of the security release window is not ideal. A bad enough vulnerability would be exploited before the window is over. And the window is ideal, but not a rule. If there’s a 0-day, any day is good for fixing the issue. If there’s some issue in the release process, late is better than waiting another week. Even under ideal conditions, we might reschedule, these sorts of things should not be hard-coded into Drupal core.
The one thing I would ask for is encouraging skewing when sites ask for updates, so we don’t have a spike of traffic at the top of an hour, its spread out throughout an hour.
- 🇬🇧United Kingdom catch
Checking frequently isn’t an issue for traffic server-side, the CDN absorbs that.
That's good to know. We previously had serious performance issues when update status checked too often, e.g. #220592: Core cache API breaks update.module: fetches data way too often, kills site performance, etc → but that issue was a long time ago.