markjaquith / WP-TLC-Transients

GNU General Public License v2.0
341 stars 37 forks source link

Addition of prevent_background option #6

Closed rosswintle closed 11 years ago

rosswintle commented 12 years ago

Hi Mark,

I've never contributed to an open source project before, so I'm starting small.

You may not want to adopt this little change/addition because using the new option wipes out most of the benefits of using TLC Transients in one fell swoop, but let me explain.

It turns out that there are some hosts out there that don't allow "loopback" HTTP requests. So the server running on http://my-domain.com can't make an HTTP request to http://my-domain.com

This has, I have found, at least two symptoms: 1) wp-cron doesn't work 2) TLC Transients doesn't work

I found this out through Twitter Widget Pro not updating correctly. A write up is here: http://oikos.org.uk/2012/03/tech-notes-twitter-widget-pro-not-updating-for-heart-internet-users/

I initially suggested to the author of Twitter Widget Pro that he make use of TLC Transients optional in his plugin, but that would mean reworking all of his transient handling code. Easier, I thought, to add a "suppress background requests" option to TLC Transients.

So that's what I've done.

Like I say, I don't mind if you don't adopt the change, but I've done the work so I thought I'd at least submit it to you for review.

As I say I've never contributed to someone else's code before so I've hope I've done it all right with the GIT Pull Request and so on.

Thanks!

Ross Wintle

aaroncampbell commented 12 years ago

I know we fight this with WordPress and wp-cron pretty regularly. The solution there is to tell the user how to have an actual cron trigger wp-cron. This would allow plugins like mine to make use of TLC-Transients (which, by the way, has been really great for efficiency and reliability) and simply add a way for a user to forcibly turn it off (add a filter, a constant, or even a gasp option). I haven't tested this patch yet, and I'm on vacation right now, but I'll try to test it out when I get back.

rosswintle commented 12 years ago

Well first thing - GO ENJOY YOUR HOLIDAY! :) (though I do appreciate your updates - you plugin writers are pretty awesome)

I did look into triggering TLC Transients using a cron job, which is the solution to my wp-cron hosting issue, but you'd need to make a POST request with the appropriate transient key so it's not a particularly simple thing to do.

Now if TLC Transients could hook into WP-Cron to do its updates...? That would be a fix. But it's a bigger piece of work.

markjaquith commented 12 years ago

If you're having this problem you're also having problems with wp-cron, since it's pretty much identical in approach. So maybe I could detect WP_ALTERNATE_CRON usage, and assume that in that case it's because self-connections don't work. I could then just tie regeneration to the next cron run. Alternately, do my own alternate cron sort of thing.

rosswintle commented 12 years ago

Hi Mark. Thanks for looking at this. You're exactly right, I am also having problems with wp-cron. However, my fix for wp-cron is to call wp-cron.php using an actual cron job, rather than using wp_alternate_cron. As I say, I could use a real cron job to call tlc_transients, but I'd need to make a POST request that included the transient key(s), which isn't simple or scalable.

Could you not use wp_cron to schedule your transient updates, rather then your own TLC_Transient_Update_Server class? You wouldn't be able to have such fine-grained control over the scheduling as you need to tie into a cron schedule to do updates, but it would mean that there was only one mechanism for doing background "jobs" in WP. I guess that would be my ideal solution. Perhaps there's scope for a fork of TLC Transients that does things this way?

Thanks

markjaquith commented 11 years ago

I could use wp_cron to schedule the updates, but it would slow down the generation and introduce the possibility of the action not completing. One of the nice things about "forking" off an HTTP request for each update is that it's the only thing the update is doing, and it is done immediately, so you get a higher level of reliability and speed of updating.

Since this is a somewhat advanced tool, that is purposefully not available in a plugin, I'm inclined to say that people who are using it and running into this problem should look into fixing the loopback issue.