Deprecate body_bytes to merge into body

[LiraNuna]

As suggested in #1736 (comment), this PR deprecates body_bytes and merges it into body.

Please note that I have intentionally removed the body_bytes param completely to elicit discussion about how to properly deprecate it (cc @ahopkins who has some thoughts to share).

[codecov]

Codecov Report

Merging #1739 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1739   +/-   ##
=======================================
  Coverage   92.18%   92.18%           
=======================================
  Files          22       22           
  Lines        2238     2238           
  Branches      419      419           
=======================================
  Hits         2063     2063           
  Misses        136      136           
  Partials       39       39

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc5925b...6fe26fc. Read the comment docs.

[Tronic]

Check out #1674 as there is some overlap, in particular with how bytes vs. other types are detected.

[LiraNuna]

Sure, I can upgrade body to bytes but that will keep the weirdness that is the two parameters merging into the same thing.

Here are the questions I have in a more blunt form:

• How will the developer know to stop using body_bytes and to start using body?
• When will body_bytes be removed?
• What happens if both body and body_bytes are passed in?
• And what's up with black adding comma to line 208?

[codecov]

Codecov Report

Merging #1739 into master will increase coverage by 0.10%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1739      +/-   ##
==========================================
+ Coverage   91.52%   91.63%   +0.10%     
==========================================
  Files          27       27              
  Lines        2998     3000       +2     
  Branches      544      545       +1     
==========================================
+ Hits         2744     2749       +5     
+ Misses        174      172       -2     
+ Partials       80       79       -1     

Impacted Files	Coverage Δ
sanic/response.py	98.08% <100.00%> (+0.02%)	⬆️
sanic/server.py	74.33% <0.00%> (+0.66%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b324ae...230e1e6. Read the comment docs.

[LiraNuna]

@Tronic, my main questions are "is it okay to deprecate this parameter?" and "how will library users know to avoid code breakage?"

I'm assuming a version bump and a note in the CHANGELOG, but is it this PR's responsibility to do that?

I think this PR is ready to review assuming the param deprecation is wanted.

[ahopkins]

Sorry for not jumping in with thoughts sooner. I was sort of waiting for the new organizational structure to kick in and let there be a bit more of a formalized discussion about how to handle deprecations.

I would like to put this one on hold until that conversation takes place. I expect the Steering Council to be in place within the next week or two.

[Tronic]

@ahopkins Any updates on that? I'd like to see this merged for 20.3.

[LiraNuna]

What is stopping this PR from being merged? I'd love to get it over the finish line

[Tronic]

As per new rules, I believe we will need a deprecation warning to anyone using body_bytes, and can only merge this one after a deprecation period in version 21.3. Could you make another PR for the warning, to be included in the upcoming 20.6 release?

[Tronic]

Alternatively, modify this PR so that it still takes body_bytes arguments but gives a warning whenever those are used.

[LiraNuna]

@Tronic how does this look? I chose to accept the old param so when the true deprecation comes, there are more options

[Tronic]

Cool, thanks. I think this should be released in 20.6 and body_bytes removed in 21.3.