Problem: FeedsHTTPFetcherResult should cache the result of the HTTP request to not issue duplicated HTTP requests during the parsing phase. I'm importing 1500 items and during different queue API runs the result will be request again although the feed has not been completely imported yet. In my case FeedsXPathParserXML needs to get the raw result, and that causes a new HTTP request.

Solution: cache the result of the HTTP request in getRaw().

CommentFileSizeAuthor
#8 feeds-http-cache-2192819-8.patch1.77 KBtwistor
#6 feeds-http-cache-2192819-5.patch1.81 KBtwistor
#5 feeds-http-fetcher-auto-scheme-2046335-5.patch5.88 KBtwistor
#3 feeds-http-cache-2192819-3.patch1.66 KBtwistor
#1 feeds-http-cache-2192819-1.patch1.23 KBklausi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Active » Needs review
FileSize
feeds-http-cache-2192819-1.patch1.23 KB

Patch attached.

twistor’s picture

twistor CreditAttribution: twistor commented
Title: FeedsHTTPFetcherResult should cache the result » FeedsHTTPFetcherResult should store the result between batches

Sorry, the title threw me a bit, since we are caching the result. I guess what's happening is that your feed source doesn't use etags are last-modified. I wonder if this will allow us to get rid of the static cache in http_request_get()?

twistor’s picture

twistor CreditAttribution: twistor commented
Category: Task » Bug report
FileSize
feeds-http-cache-2192819-3.patch1.66 KB

Keeping that in memory isn't really ideal, but we do it already multiple times, and that all needs to be seriously fixed.

I'm going to say this is a bug, because other weird things can happen if you're re-downloading something in-between cron runs.

Only real change is to use !isset() instead of empty() so that an empty response body isn't fetched more than once.

Status: Needs review » Needs work

The last submitted patch, 3: feeds-http-cache-2192819-3.patch, failed testing.

twistor’s picture

twistor CreditAttribution: twistor commented
Status: Needs work » Needs review
FileSize
feeds-http-fetcher-auto-scheme-2046335-5.patch5.88 KB

That makes sense.

twistor’s picture

twistor CreditAttribution: twistor commented
FileSize
feeds-http-cache-2192819-5.patch1.81 KB

Entirely wrong patch.

The last submitted patch, 5: feeds-http-fetcher-auto-scheme-2046335-5.patch, failed testing.

twistor’s picture

twistor CreditAttribution: twistor commented
FileSize
feeds-http-cache-2192819-8.patch1.77 KB

There's no reason to call parent::__construct() at all.

twistor’s picture

twistor CreditAttribution: twistor commented
Version: 7.x-2.x-dev » 6.x-1.x-dev
Status: Needs review » Patch (to be ported)

This is eligible for backport.

http://drupalcode.org/project/feeds.git/commit/a8468ac

twistor’s picture

twistor CreditAttribution: twistor as a volunteer commented
Status: Patch (to be ported) » Closed (outdated)