Skip to content

Commit 359b6a1

Browse files
author
Tom Augspurger
authored
Fixed retries (#55)
* Fixed retries We weren't retrying, since we didn't specify which error codes to retry on. This fixes that, and includes tests (which I should have added in the first place). We use `responses` to mock the response to requests.
1 parent 7e2b9f6 commit 359b6a1

4 files changed

Lines changed: 32 additions & 4 deletions

File tree

planetary_computer/sas.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ class SASToken(SASBase):
5858

5959
def sign(self, href: str) -> SignedLink:
6060
"""Signs an href with this token"""
61-
return SignedLink(href=f"{href}?{self.token}", expiry=self.expiry)
61+
return SignedLink(
62+
href=f"{href}?{self.token}", expiry=self.expiry # type: ignore [call-arg]
63+
)
6264

6365
def ttl(self) -> float:
6466
"""Number of seconds the token is still valid for"""
@@ -264,7 +266,6 @@ def _sign_fsspec_asset_in_place(asset: AssetLike) -> None:
264266

265267
storage_options = None
266268
for key in ["table:storage_options", "xarray:storage_options"]:
267-
268269
if key in extra_d:
269270
storage_options = extra_d[key]
270271
break
@@ -444,6 +445,7 @@ def get_token(
444445
retry = urllib3.util.retry.Retry(
445446
total=retry_total,
446447
backoff_factor=retry_backoff_factor,
448+
status_forcelist=[429, 500, 502, 503, 504],
447449
)
448450
adapter = requests.adapters.HTTPAdapter(max_retries=retry)
449451
session.mount("http://", adapter)

scripts/cibuild

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
1919
else
2020
# Install/upgrade dependencies
2121
python -m pip install --upgrade pip
22-
pip install -r requirements-dev.txt
23-
pip install -e .[adlfs,azure]
22+
pip install -e .[adlfs,azure,dev]
2423

2524
./scripts/test
2625
fi

setup.cfg

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ install_requires =
2222
[options.extras_require]
2323
adlfs = adlfs
2424
azure = azure-storage-blob
25+
dev =
26+
black
27+
flake8
28+
mypy
29+
types-requests
30+
setuptools
31+
pytest
32+
responses
2533

2634
[options.entry_points]
2735
console_scripts =

tests/test_signing.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
from pathlib import Path
66
import warnings
77
import pystac
8+
import pytest
9+
from requests.exceptions import RetryError
810

11+
import responses
912
import requests
1013

1114
import planetary_computer as pc
@@ -367,3 +370,19 @@ def test_is_fsspec_url(self) -> None:
367370

368371
asset = Asset("adlfs://my-container/my/path.ext")
369372
self.assertFalse(is_fsspec_asset(asset))
373+
374+
375+
@responses.activate
376+
def test_retry() -> None:
377+
TOKEN_CACHE.clear()
378+
rsp1 = responses.Response(
379+
method="GET",
380+
url="https://planetarycomputer.microsoft.com/api/sas/v1/token/naipeuwest/naip",
381+
status=503,
382+
)
383+
responses.add(rsp1)
384+
385+
with pytest.raises(RetryError):
386+
get_token("naipeuwest", "naip")
387+
388+
assert rsp1.call_count == 11

0 commit comments

Comments
 (0)