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

Change cron from hourly to twicedaily fixes #1242 #1246

Merged
merged 3 commits into from Jun 28, 2018
Merged

Conversation

dac514
Copy link
Contributor

@dac514 dac514 commented Jun 27, 2018

Based on: https://core.trac.wordpress.org/ticket/44370

Many of the suggestions in the WP trac ticket seem non trivial to carry out. Performance implications seem exaggerated.

Downloads are direct links. Clicking on them doesn't trigger any action. The whole mechanism is actually sort of insecure. It relies on obfuscation. Unless we check Nginx logs we can't know when a user has downloaded.

The function that does all the work. the alleged performance hog, is wp_privacy_delete_old_export_files. This function scans a dir and deletes files that are older than 3 days. The 3 days corresponds to the text sent in an email. The amount of days can be filtered by wp_privacy_export_expiration, but I guess my point is we can't change anything except when files get deleted. The cron job still needs to run.

Since there isn't anything except personal data files in exports ./wp-personal-data-exports/ I don't actually think this will cause significant performance problems?

I've changed the cron job from hourly to twicedaily which should good enough to solve "runs too much" IMHO.

privacy-exports-1

privacy-exports-2

@greatislander greatislander self-assigned this Jun 27, 2018
@greatislander greatislander added this to the 5.4.0 milestone Jun 27, 2018
Copy link
Contributor

@greatislander greatislander left a comment

Choose a reason for hiding this comment

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

This looks good, but in keeping with a5e1b8f, which introduced Pressbooks\Admin\Privacy, I'd like this class to be Pressbooks\Privacy (it supports general privacy functionality, not just GDPR).

@dac514 dac514 changed the title Chang cron from hourly to twicedaily fixes #1242 Change cron from hourly to twicedaily fixes #1242 Jun 28, 2018
@codecov
Copy link

codecov bot commented Jun 28, 2018

Codecov Report

Merging #1246 into dev will increase coverage by 0.02%.
The diff coverage is 88.88%.

@@             Coverage Diff              @@
##                dev    #1246      +/-   ##
============================================
+ Coverage     59.16%   59.19%   +0.02%     
- Complexity     3760     3769       +9     
============================================
  Files            97       97              
  Lines         16446    16460      +14     
============================================
+ Hits           9730     9743      +13     
- Misses         6716     6717       +1

@greatislander greatislander merged commit 25221fd into dev Jun 28, 2018
@greatislander greatislander deleted the gdpr-cron-scaling branch June 28, 2018 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants