While reviewing #3041885: Display relevant Security Advisories data for Drupal → , @catch called out an implementation detail in https://git.drupalcode.org/project/drupal/-/merge_requests/284#note_22874 (emphasis mine, to show the potential benefit):
Relying on key value expire for the data to be refreshed means http requests on admin pages when it's empty (only once every 12 hours, but on a low-admin-traffic site that could be the first admin request every visit).
I think we could do the following - would be more robust in the case the feed is down and no impact on admin page views:
- In system_cron(), poll Drupal.org. Keep the fallback though in case the entry is empty.
- Use the flood system to rate limit the polling requests on cron, so that it respects the interval setting - so each hook_cron() call finishes early if flood acquire fails and still only does the actual work every 12 hours.
- When the interval is changed in config to a lower value, instead of deleting the data, we could clear the flood record - this will still force an update next cron run but means the current data is preserved in the meantime.
- We could set the expire in k/v to interval * 2 or interval * 1.5 so that it does still eventually expire if outbound requests consistently fail.
N/A
Discuss if it's worth doing what @catch suggests, and implement it if so.
Decide if we want to make this change, and if we do, write a merge request with tests. Then commit it.
None.
None. This functionality is not an API.
None.
TBD, but likely none.
Active
11.0 🔥
system.module
Not all content is available!
It's likely this issue predates Contrib.social: some issue and comment data are missing.
In a discussion on the original MR @tedbow pointed out that \Drupal\system\SecurityAdvisories\SecurityAdvisoriesFetcher::getSecurityAdvisories is called with $allow_outgoing_request = FALSE in hook_page_top. When it's called from hook_cron and hook_requirements this param is TRUE.
If I understand correctly, @catch originally suggested we add flood control to system_cron, but in light of the above I think we can put it in ::getSecurityAdvisories when the bool is TRUE.