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

typing: Account for the fact that requests.HTTPError .response migth be None now #1336

Merged
merged 1 commit into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dandi/dandiapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,10 @@ def request(
if isinstance(e, requests.HTTPError):
lgr.error(
"HTTP request failed repeatedly: Error %d while sending %s request to %s: %s",
e.response.status_code,
e.response.status_code if e.response is not None else "?",
method,
url,
e.response.text,
e.response.text if e.response is not None else "?",
)
else:
lgr.exception("HTTP connection failed")
Expand Down
12 changes: 10 additions & 2 deletions dandi/dandiarchive.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,11 @@
try:
dandiset = self.get_dandiset(client, lazy=not strict)
except requests.HTTPError as e:
if e.response.status_code == 401 and authenticate is not False:
if (

Check warning on line 168 in dandi/dandiarchive.py

View check run for this annotation

Codecov / codecov/patch

dandi/dandiarchive.py#L168

Added line #L168 was not covered by tests
e.response is not None
and e.response.status_code == 401
and authenticate is not False
):
lgr.info("Resource requires authentication; authenticating ...")
client.dandi_authenticate()
dandiset = self.get_dandiset(client, lazy=not strict)
Expand Down Expand Up @@ -293,7 +297,11 @@
try:
assets = list(self.get_assets(client, strict=strict))
except requests.HTTPError as e:
if e.response.status_code == 401 and authenticate is not False:
if (

Check warning on line 300 in dandi/dandiarchive.py

View check run for this annotation

Codecov / codecov/patch

dandi/dandiarchive.py#L300

Added line #L300 was not covered by tests
e.response is not None
and e.response.status_code == 401
and authenticate is not False
):
lgr.info("Resource requires authentication; authenticating ...")
client.dandi_authenticate()
assets = list(self.get_assets(client, strict=strict))
Expand Down
12 changes: 8 additions & 4 deletions dandi/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -700,10 +700,14 @@
except requests.exceptions.HTTPError as exc:
# TODO: actually we should probably retry only on selected codes, and also
# respect Retry-After
if attempt >= 2 or exc.response.status_code not in (
400, # Bad Request, but happened with gider:
# https://github.com/dandi/dandi-cli/issues/87
*RETRY_STATUSES,
if attempt >= 2 or (

Check warning on line 703 in dandi/download.py

View check run for this annotation

Codecov / codecov/patch

dandi/download.py#L703

Added line #L703 was not covered by tests
exc.response is not None
and exc.response.status_code
not in (
400, # Bad Request, but happened with gider:
# https://github.com/dandi/dandi-cli/issues/87
*RETRY_STATUSES,
)
):
lgr.debug("Download failed: %s", exc)
yield {"status": "error", "message": str(exc)}
Expand Down
2 changes: 1 addition & 1 deletion dandi/files/bases.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ def iter_upload(
},
)
except requests.HTTPError as e:
if e.response.status_code == 409:
if e.response is not None and e.response.status_code == 409:
lgr.debug("%s: Blob already exists on server", asset_path)
blob_id = e.response.headers["Location"]
else:
Expand Down
2 changes: 1 addition & 1 deletion dandi/files/zarr.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ def mkzarr() -> str:
json={"name": asset_path, "dandiset": dandiset.identifier},
)
except requests.HTTPError as e:
if "Zarr already exists" in e.response.text:
if e.response is not None and "Zarr already exists" in e.response.text:
lgr.warning(
"%s: Found pre-existing Zarr at same path not"
" associated with any asset; reusing",
Expand Down