Skip to content

Retry GET requests#87

Merged
ttung merged 1 commit intomasterfrom
tonytung-retry
Jan 10, 2018
Merged

Retry GET requests#87
ttung merged 1 commit intomasterfrom
tonytung-retry

Conversation

@ttung
Copy link
Copy Markdown
Member

@ttung ttung commented Jan 10, 2018

  1. Add a retry policy class that dictates which requests are retried and how often they are retried.
  2. Add a test that uses the fake 504 mechanism in data-store to test the retry policy.

Connects to HumanCellAtlas/data-store#485

@ttung ttung requested a review from kislyuk January 10, 2018 16:23
@ghost ghost assigned ttung Jan 10, 2018
@ghost ghost added the code review label Jan 10, 2018
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 10, 2018

Codecov Report

Merging #87 into master will decrease coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
- Coverage   87.66%   87.54%   -0.13%     
==========================================
  Files          28       28              
  Lines         924      931       +7     
==========================================
+ Hits          810      815       +5     
- Misses        114      116       +2
Impacted Files Coverage Δ
hca/util/__init__.py 90.2% <100%> (-0.98%) ⬇️
hca/dss/upload_to_cloud.py 70.58% <0%> (+1.96%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85e0596...1c187a2. Read the comment docs.

@hannes-ucsc
Copy link
Copy Markdown
Contributor

Let me plug https://github.com/HumanCellAtlas/data-store/blob/master/dss/util/retry.py#L14 here which could also be used for this if it weren't in a different project.

@ttung
Copy link
Copy Markdown
Member Author

ttung commented Jan 10, 2018

Let me plug https://github.com/HumanCellAtlas/data-store/blob/master/dss/util/retry.py#L14 here which could also be used for this if it weren't in a different project.

time for HumanCellAtlas/dcp-py-common

Copy link
Copy Markdown
Member

@kislyuk kislyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

urllib3 has retry logic built in. This retry policy logic can be customized and accessed via requests via the max_retries kwarg, to which you can pass a urllib3.Retry() object, which you can configure or subclass to customize this behavior.

Let's switch to using that functionality.

@ttung ttung force-pushed the tonytung-retry branch 2 times, most recently from e8982d0 to af08821 Compare January 10, 2018 20:11
@ttung ttung requested a review from kislyuk January 10, 2018 20:11
@ttung
Copy link
Copy Markdown
Member Author

ttung commented Jan 10, 2018

Updated to use urllib3 retry.

@ttung ttung changed the base branch from tonytung-fqid to master January 10, 2018 20:15
1. Add a retry policy class that dictates which requests are retried and how often they are retried.
2. Add a test that uses the fake 504 mechanism in data-store to test the retry policy.

Connects to HumanCellAtlas/data-store#485
Copy link
Copy Markdown
Member

@kislyuk kislyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ttung ttung merged commit 9919356 into master Jan 10, 2018
@ghost ghost removed the code review label Jan 10, 2018
@ttung ttung deleted the tonytung-retry branch January 21, 2018 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants