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 #1335

Closed
wants to merge 1 commit into from
Closed
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 else "?",
method,
url,
e.response.text,
e.response.text if e.response 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
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
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
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 @@
},
)
except requests.HTTPError as e:
if e.response.status_code == 409:
if e.response and e.response.status_code == 409:

Check warning on line 365 in dandi/files/bases.py

View check run for this annotation

Codecov / codecov/patch

dandi/files/bases.py#L365

Added line #L365 was not covered by tests
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 @@
json={"name": asset_path, "dandiset": dandiset.identifier},
)
except requests.HTTPError as e:
if "Zarr already exists" in e.response.text:
if e.response and "Zarr already exists" in e.response.text:

Check warning on line 302 in dandi/files/zarr.py

View check run for this annotation

Codecov / codecov/patch

dandi/files/zarr.py#L302

Added line #L302 was not covered by tests
lgr.warning(
"%s: Found pre-existing Zarr at same path not"
" associated with any asset; reusing",
Expand Down
Loading