After setting up my site-wide flickr user id and creating two new photosets on Flickr, i enabled the Recent Photosets block. It worked, pulling in the two sets in the block. But when I tried to change the set cover photo on Flickr and it didn't update on my Drupal site, I thought I would delete that set and create another one in its place. Now both sets shown in the block have broken links and the set cover photo remains as it was when I first enabled the block.

Comments

anj’s picture

I'd like to bump this issue, as I have tried installing both the 1.0 and 1.x iterations for the flickr module for Drupal 5, and neither seems to update properly. The lists of sets and photos have remained fixed, and have not been updated to reflect changes at flickr since the time the pages were enabled. This applies to all blocks, the site page (/flickr) and the user pages (/flickr/1 etc).

I suspect the caching system is broken, as the configuration parameter never updates. I'll just have a look...

Right, I just hacked off the caching system (sticking a '$cacheable = FALSE;' in flickr.inc/flickr_request) and everything started to update sensibly. There is some problem with the caching system, but I don't have time to finish debugging this right now, sorry.

Hope that helps.

Anj

anj’s picture

Ok, well I looked into it anyway, and found that this was a 'feature'. The expiry time for the cache of flickr API calls is only advisory, and the caching system just ensures that the cache is kept for at least that long. The data will only get refreshed on the next cache wipe, which could take an awful long time if you use a reasonably aggressive caching policy.

The fix is simple. To enforce the cache time limit, instead of it being advisory, the line

      return unserialize($cache->data);

in flickr.inc is made conditions, such that:

      if( $cache->expire > time() ) {
        return unserialize($cache->data);
      }

Thus, the cached data is only used if it is still 'in date'.

Hope that helps.

Anj

drewish’s picture

the cache gets cleared when you create a node (or maybe even posting a comment). adding an explicit check isn't a bad idea.

andrewlevine’s picture

For the most control of caching, there would be two configuration options: cache for at least x minutes and cache for no more than x minutes.

However, I get the feeling that more non-standard caching options wouldn't be used much and would create confusion. We are using the standard drupal caching mechanism. Shouldn't some other caching module have the job of tweaking this functionality?

So I'd rather keep it as it is unless there is something I am missing.

anj’s picture

I think the issue is that the internal cache system can know when it has fallen out of date, i.e. it can update the cache when a node is edited because it is all being done within Drupal.

With external content, the ideal update frequency is probably 'whenever the external source changes', but for many practical reasons the updates are configured to happen less frequently (bandwidth use etc). Looking at the code this way, the flickr module works no differently from the aggregator - drawing in content from an external feed. Therefore I suggest that the flickr module uses the same approach (and language) as the aggregator. IIRC, this means that we should configure an 'update frequency' instead of a 'caching time', although under the hood these are essentially the same thing, of course.

Anj

anj’s picture

Component: flickr_sets » flickr (main module)
StatusFileSize
new1.62 KB

Here's my suggestion in patch form. I hope you'll agree that following the aggregator mould is the path of least confusion, but I understand if you prefer your approach.

Thanks for your time.

Anj

drewish’s picture

i'm partial to andy j's solution but i'd like to get andrewlevine's feedback.

andrewlevine’s picture

I like andy j's solution best too. The patch looks fine although I haven't actually tested it.

anj’s picture

Status: Active » Needs review

It's a patch against the DRUPAL-5 branch, which is hopefully the right one. I've tested in on my own site, but that's all. More testing is wise, so I'm changing the status of this issue to 'patch (code needs review)' - I hope that's ok.

Andy J.

drewish’s picture

Status: Needs review » Fixed
StatusFileSize
new1.69 KB

okay, i gave it a whirl and it looks good the only question i've got is does it require crontab? i don't think it does... so i've added one comment and removed that bit from the description. this is a simple enough feature that it's not worth dragging out any longer. i'm going to commit this. if there's something you don't like, re-open this and we can follow up.

oh, a quick note on the issue status, any time there's a patch attached and in consideration, feel free to flip it to "needs review". if someone looks at it and doesn't like it they'll change it to "needs work". if you review someone else's code, it looks good and tests out, then mark it as "ready to be committed". in core the standard for RTBC is a bit higher than in most contrib modules.

anj’s picture

Ah, yes, you're completely right. Crontab is not required, as old data will be refreshed on demand. Sorry about that.

Cheers,
Andy J.

drewish’s picture

oh, i forgot to say thanks! it's very nice to have people point out your bugs and then submit a patch.

anj’s picture

No worries - It's good to be able to help out. Thanks again.

Anonymous’s picture

Status: Fixed » Closed (fixed)
justinledwards’s picture

Is this going to be added to a release soon?

How do you apply patches?

I downloaded the flickr module yesterday for 5.X and I am still having this issue.