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

[HttpClient] added extra.trace_content option to TraceableHttpClient to prevent it from keeping the content in memory #38587

Merged
merged 1 commit into from
Oct 16, 2020

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 5.2
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

TraceableHttpClient leaks memory by definition. But sometimes, this leak is to important, especially when keeping the response content in memory.

This PR adds a new trace_content option under extra so that consumers can tell the client to not trace the content. This will be ignored when TraceableHttpClient is not in use.

…nt` to prevent it from keeping the content in memory
@stof
Copy link
Member

stof commented Oct 15, 2020

should we have an option in the TraceableHttpClient constructor to control the default value rather than harcoding it to true ?

@nicolas-grekas
Copy link
Member Author

We could have an option in the constructor, and a corresponding config option.
That's separate from this proposal of course.

@fabpot
Copy link
Member

fabpot commented Oct 16, 2020

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 72be305 into symfony:5.x Oct 16, 2020
@nicolas-grekas nicolas-grekas deleted the hc-dont-trace-content branch October 20, 2020 11:30
Nyholm added a commit that referenced this pull request Oct 22, 2020
… (nicolas-grekas)

This PR was merged into the 5.x branch.

Discussion
----------

[HttpClient] never trace content of event-stream responses

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Let's leverage #38587 in `EventSourceHttpClient`

Commits
-------

e4c0262 [HttpClient] never trace content of event-stream responses
@fabpot fabpot mentioned this pull request Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants