Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add retry mechanism to `fetch_text` #85

Merged
merged 1 commit into from Apr 19, 2019

Conversation

Projects
None yet
2 participants
@rgreinho
Copy link
Member

commented Apr 19, 2019

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Description

Sometimes the fetch_text function does not succeed to retrieve data
and makes ScrAPD crash. This patch adds a retry mechanism to this
function, with exponential backoff, and simply raises a ValueError
exception if it really cannot retrieve any data.

Checklist:

  • [] I have updated the documentation accordingly
  • [] I have written unit tests

Fixes #83

@rgreinho rgreinho self-assigned this Apr 19, 2019

@rgreinho rgreinho requested a review from mrengler Apr 19, 2019

@rgreinho rgreinho force-pushed the rgreinho:issues/83/fecth-page-retry branch 3 times, most recently from 3d234b2 to a4a4f24 Apr 19, 2019

Add retry mechanism to `fetch_text`
Sometimes the `fetch_text` function does not succeed to retrieve data
and makes ScrAPD crash. This patch adds a retry mechanism to this
function, with exponential backoff, and simply raises a `ValueError`
exception if it really cannot retrieve any data.

Fixes #83

@rgreinho rgreinho force-pushed the rgreinho:issues/83/fecth-page-retry branch from a4a4f24 to c72dad8 Apr 19, 2019

@@ -15,6 +18,7 @@
PAGE_DETAILS_URL = 'http://austintexas.gov/'


@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=4, max=10))

This comment has been minimized.

Copy link
@mrengler

mrengler Apr 19, 2019

Contributor

Have we seen any examples of >3 failures? I'm not sure how rare it is but that seems safe to me.

This comment has been minimized.

Copy link
@rgreinho

rgreinho Apr 19, 2019

Author Member

We have not because it currently fails at the first one. If we see more failures, I'll readjust the parameters (more retries, more wait).

@mrengler
Copy link
Contributor

left a comment

Retry functionality and tests look good to me, and tests / formatting passes.

@mergify mergify bot merged commit 10b0187 into scrapd:master Apr 19, 2019

7 checks passed

Mergify — Summary 1 rule matches
Details
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: format Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: prepare Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
coverage/coveralls Coverage increased (+0.4%) to 97.887%
Details

@rgreinho rgreinho deleted the rgreinho:issues/83/fecth-page-retry branch Apr 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.