Httpx Response content is not set/read, causing JSON parsing errors #834
Comments
16 tasks
Yun-Kim
added a commit
to DataDog/dd-trace-py
that referenced
this issue
Jul 30, 2024
This PR attempts to address the flakiness and general (lack of) reliability of the LangChain test suite, and removes the flaky test markers from most langchain tests. No functionality has been changed, this PR focuses only on improving our test suite to be less flaky and be a better signal. Problems with LangChain test suite: 1. Tests are randomly flaking due to cassettes being read incorrectly (See related [issue](kevin1024/vcrpy#834)) 2. Tests are completely skipped except for patch tests. This seems like something to do with our skipping logic by looking at langchain versions and Python versions (we skip a few test cases in Python 3.9 due to unnecessary cassette files that are required specifically for 3.9). This PR solves the above problems by: 1. Pinning `vcrpy` to version `5.1.0` which is the version prior to the linked issue being introduced 2. Rewriting the langchain version skipping logic to rely specifically on the Langchain module version (instead of reusing the `PATCH_LANGCHAIN_V0` constant from the patch file which appeared to not be truthful) 3. Change the Python version skip checks to only use major/minor versions rather than checking against `(3, 10, 0)` as this may result in some edge cases. Do not skip Python 3.9 testing on `test_langchain_community.py` or community tests on `test_langchain_llmobs.py`. ## Checklist - [x] The PR description includes an overview of the change - [x] The PR description articulates the motivation for the change - [x] The change includes tests OR the PR description describes a testing strategy - [x] The PR description notes risks associated with the change, if any - [x] Newly-added code is easy to change - [x] The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - [x] The change includes or references documentation updates if necessary - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Newly-added code is easy to change - [x] Release note makes sense to a user of the library - [x] If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi there, running into the same issue as #832. It seems like getting rid of the manual step to set
response._content, as well as mocking thehttpx.Response.read()function resulted in the following error by httpx (occurs non-deterministically in my CI):I haven't narrowed down the non-deterministic nature of this bug, but it seems like there's two solutions to this:
response._contentthat was removed by Make httpx_stubs generate cassettes consistent with other stubs. #649httpx.Response.read(not sure what the implications of this is)This is causing a good amount of flakiness in my CI, so would really appreciate any insights/help getting this fixed. Thanks!
The text was updated successfully, but these errors were encountered: