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

Simplify Response object #533

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jsha
Copy link
Collaborator

@jsha jsha commented Jul 27, 2022

Now that we create a Reader immediately on construction, we can get rid of unit, length, and compression by passing the necessary parameters directly to stream_to_reader. This also lets us avoid temporarily making a half-constructed Response so we can call stream_to_reader on it. Now stream_to_reader is an associated method that doesn't take Self so it can be called during construction.

Followup to #531.

jsha and others added 2 commits July 26, 2022 23:15
Now that we create a Reader immediately on construction, we can get
rid of `unit`, `length`, and `compression` by passing the necessary
parameters directly to stream_to_reader.
@algesten
Copy link
Owner

@jsha In my mind, the whole reason for Unit is to encapsulate the request arguments. I think this PR does do a nice cleanup, but we could use Unit as an argument into stream_to_reader. I pushed a commit to that effect.

Reading the code there is another problem.

  1. Here we remove content-length from headers
  2. We pass headers to stream_to_reader
  3. Then we try to parse content-length from headers in stream_to_reader.

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