Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
aggregator_aggregator_fetch() updates the feed record when done fetching. This should be handled by the Aggregator API.
Without this Aggregator is not ready for prime time.
Comments
Comment #1
alex_b CreditAttribution: alex_b commentedRight. Subscribing.
Comment #2
mustafau CreditAttribution: mustafau commentedComment #4
mustafau CreditAttribution: mustafau commentedAPI clean-up
Comment #5
alex_b CreditAttribution: alex_b commentedDoes #2 need to be applied together with #402280-6: Parser should not update aggregator_feed record ?
Comment #6
mustafau CreditAttribution: mustafau commentedNo. Tests were ok when I submitted this patch.
Comment #7
mustafau CreditAttribution: mustafau commentedComment #8
mustafau CreditAttribution: mustafau commentedComment #9
Dries CreditAttribution: Dries commentedSmall indentation issue -- two spaces too many.
Comment #10
alex_b CreditAttribution: alex_b commentedPatch looks good.
I fixed the indentation issue.
RTBC
Comment #11
alex_b CreditAttribution: alex_b commentedTaking another look, I see this patch is creating a soft expectation that the fetcher uses HTTP (http_response) - we should avoid assumptions about the fetching method aggregator.module .
A simple TRUE or FALSE as return value of the fetching stage would be sufficient feedback for making aggregator invoke the parsing stage or not.
Will take a stab at this.
Comment #12
alex_b CreditAttribution: alex_b commented- Adds a TRUE/FALSE return value to fetching stage (consistent with parsing stage)
- Renders check for http_response obsolete, thus removes the HTTP assumption in aggregator.module that #10 would have introduced.
Further:
- Removes duplicate computation of md5 hash
- Removes some minimal white space from api.php.
Comment #15
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!