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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,11 @@ def navigate(
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 (
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 @@ def navigate(
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 (
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,10 +700,14 @@ def _download_file(
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 (
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
View file
Open in desktop
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
View file
Open in desktop
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