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

Remove deprecated notice and add preload command #1

Merged
merged 2 commits into from Dec 16, 2013

Conversation

iandunn
Copy link
Contributor

@iandunn iandunn commented Dec 15, 2013

No description provided.

Documentation is created inline now, so this is no longer needed, and if it's present a warning is output: Warning: WP_CLI::add_man_dir() is deprecated. Add docs inline.
@iandunn
Copy link
Contributor Author

iandunn commented Dec 15, 2013

Err, I should have made separate branches for those and submitted them individually. Let me know if you want to just accept one or the other and I'll do that.

@scribu
Copy link
Member

scribu commented Dec 15, 2013

A single PR is fine in this case.

Question: why do you schedule a cron job instead of calling the hook directly?

@iandunn
Copy link
Contributor Author

iandunn commented Dec 16, 2013

wp_cron_preload_cache() runs in batches, caching 100 items at a time, and then, if there are more items to process, it schedules another cron job in 30 seconds to run the next batch, and so on until it finishes processing all the items.

So, calling it directly would only do the first batch while the command is running, and the rest of the batches would still finish via cron.

Also, my use case for this is that I have a script that runs through all the domains in my hosting account and uses WP-CLI to upgrade Core/plugins/themes. I want to also flush and preload the cache for each domain after the upgrades run.

I figured if I call wp_cron_preload_cache() directly, it'd take 5-20 minutes per domain to finish preloading before moving on to the next domain, which doesn't really make sense in this context. I just want to kick off the process, and then move on.

I guess I could modify the command to call wp_cron_preload_cache() directly, then detect if there are more items and unschedule the job that it scheduled, then call it again, N times until it finishes, but that doesn't seem necessary or desirable to me. What do you think?

@scribu
Copy link
Member

scribu commented Dec 16, 2013

Ok, that makes sense. Merging.

scribu pushed a commit that referenced this pull request Dec 16, 2013
Remove deprecated notice and add preload command
@scribu scribu merged commit ada11fa into wp-cli:master Dec 16, 2013
@scribu
Copy link
Member

scribu commented Dec 16, 2013

To clarify, I mean that if you already have a system for doing it asynchronously, it doesn't make sense to make it blocking.

Thanks for the PR.

@iandunn
Copy link
Contributor Author

iandunn commented Dec 16, 2013

No problem :)

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.

None yet

2 participants