Conversation
|
Is this a flaky test? I reverted my changes locally and it looks like |
geruh
left a comment
geruh
left a comment
There was a problem hiding this comment.
Hello, thanks for raising this! Seems like the spec doesn't follow the standard for head request as failures in the spec have response models. But I believe here we could at least align with what happens in java today.
When the responseBody is null (following what you're saying here), JSON skips parsing and falls back to buildDefaultErrorResponse() which
uses the HTTP reason to construct the error. I believe the only difference here is in Python, the exception type is resolved first, then we try to build a message.
We can match that expected behavior and add some additional tests for empty responses. Wdyt?
As for the test failure it does look unrelated... let me try to run them locally
| response = f"{error.type}: {error.message}" | ||
| # Handle empty response bodies (Specifically HEAD requests via exist requests) | ||
| if not exc.response.text: | ||
| response = f"{exception.__name__}: {exc.response.reason}" |
There was a problem hiding this comment.
Nit: the exc.response.reason can be None
# Rationale for this change While reviewing some open PRs #3041, #3042, I noticed CI kept failing on the Python 3.13 job in the hive tests. Turns out [CPython 3.13.12 ](https://docs.python.org/3.13/whatsnew/changelog.html#id3)was just released and included a change python/cpython#142651 which made `Mock.call_count` thread-safe by deriving it from `len(call_args_list)`. This broke our hive test, which was resetting the counter with `mock.call_count = 0` directly. This switches to use reset_mock(), which properly clears all the internal call tracking state. ## Are these changes tested? `make test` passes ## Are there any user-facing changes? no
2 participants
Rationale for this change
HEAD responses return with an empty body, therefore when we try to use pydantic to parse the json body, it fails on an empty string. This bypasses pydantic if the body does not exist.
Before:
After:
Are these changes tested?
Yes
Are there any user-facing changes?
No