Problem

  • Domains are marked as invalid when the API request fails, which can cause valid domains to be invalidated.
  • Whilst testing this behaviour, I noticed that allowed domain statuses are cached permanently and are never invalidated as the `skimlinks_domain_allowed:` cache isn't cleared.

Proposed resolution

  • Keep the exising domain status when the API requests fails (skip update), as it will be retried later.
  • Clear the `skimlinks_domain_allowed:` cache when a domain is updated.
CommentFileSizeAuthor
#2 2986567_api_failure.patch4.25 KBdavidgrayston

Comments

davidgrayston created an issue. See original summary.

davidgrayston’s picture

davidgrayston commented
StatusFileSize
new 2986567_api_failure.patch4.25 KB

Patch attached:

  • Clears allowed domain cache when domains are updated
  • Skips domain update when CURL error occurs
  • Skips domain update when API response code is not 200
  • Stops cron before hitting rate limit of 40 requests per minute (within same process)
  • Stops cron process when 406 response code is returned (hit API rate limit) - see http://developers.skimlinks.com/merchant.html – this could occur if cron is run in more than one process and/or run more than once per minute.
davidgrayston’s picture

davidgrayston commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 2986567_api_failure.patch, failed testing. View results

marcelovani’s picture

marcelovani
Primary language Portuguese, Brazil
Location London
commented

Tests are failing because of #2986821: Skimlinks API changed

marcelovani’s picture

marcelovani
Primary language Portuguese, Brazil
Location London
commented
Status: Needs work » Fixed

Thanks for the patch David
I will fix the tests on the respective issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.