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

Save memory by not storing full request history #170

Merged
merged 2 commits into from Apr 3, 2017
Merged

Conversation

moritz
Copy link
Collaborator

@moritz moritz commented Apr 1, 2017

See #169.

We used to store the full history of the response, just to count redirects.
This used up lots of RAM for very little gain.

This commit replaces it by a simple integer that stores the count of
redirects in a row.

Since @.history was a public attribute, this changes the public API,
and as such is subject to discussion.

See #169.
We used to store the full history of the response, just to count redirects.
This used up lots of RAM for very little gain.

This commit replaces it by a simple integer that stores the count of
redirects in a row.

Since @.history was a public attribute, this changes the public API,
and as such is subject to discussion.
@jonathanstowe
Copy link
Collaborator

I'm pretty relaxed about the public API part as history is nether documented nor explicitly tested for (only the re-direct loop detection behaviour is tested,) If, for some reason, there was a need to find the re-direct chain that led to a specific response then this would be better placed in the response (and as the request is already saved in the response, this may be trivial.)

So as long as @ugexe is happy (for it is he who put the redirect limit in originally,) and @sergot doesn't explicitly object, I'll merge this.

@jonathanstowe jonathanstowe mentioned this pull request Apr 2, 2017
@jonathanstowe
Copy link
Collaborator

As no-one has raised any objections and in lieu of a better suggestion I am going with this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants