Implement aiohttp.ClientResponse.ok property

[EvanKepner]

What do these changes do?

This adds ClientResponse.ok as a property, similar to requests.Response.ok to help people moving code to async.

Are there changes in behavior for the user?

Not directly. Users will get a new property for ok to check if the status is under 400.

Related issue number

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes
• If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • The format is <Name> <Surname>.
  • Please keep alphabetical order, the file is sorted by names.
• Add a new news fragment into the CHANGES folder
  • name it <issue_id>.<type> for example (588.bugfix)
  • if you don't have an issue_id change it to the pr id after creating the pr
  • ensure type is one of the following:
    • .feature: Signifying a new feature.
    • .bugfix: Signifying a bug fix.
    • .doc: Signifying a documentation improvement.
    • .removal: Signifying a deprecation or removal of public API.
    • .misc: A ticket has been closed, but it is not of interest to users.
  • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

[webknjaz]

@EvanKepner please update the docs @ https://github.com/aio-libs/aiohttp/blob/master/docs/client_reference.rst and I'll be able to merge this.

[EvanKepner]

@webknjaz thanks for the fast review and the assist on the test structure! For docs - is this a manual edit on that file or built some other way through sphinx? I tried make doc from the top level and there were no files changed to commit after the pages rebuild. If it's a manual edit I'll push a new commit but if it should be auto-generated some other way let me know.

[webknjaz]

Yes, just change that RST file.

[webknjaz]

Perfect!

[webknjaz]

@EvanKepner I cannot debug how to make it write in a browser. Could you please fix this locally?

[EvanKepner]

Yup, no problem - pushed a commit and it should pass CI now. ids is an optional arg, and with the structure of the tuple for parameterization I think pytest is unpacking as a list of lists which makes the lambda function difficult. I dropped ids entirely to use the default values and changed the first arg to be a single string.

The other way around this would be to do something like:

from typing import NamedTuple

class OKCases(NamedTuple):
    status: int
    expected_ok: boolean
    
    @property 
    def idfn(self):
        return f"{self.status} is {self.expected_ok}"

For the parameterized tuples to access as a single value but that feels excessive just for ids that already print the values.

[webknjaz]

Well, the previous version of lambda relied on unpacking but still was breaking in the CI somehow. Thanks for the fix anyway

[webknjaz]

As for the param names, I prefer them to be a tuple of strings for readability

[codecov-io]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.60%. Comparing base (3dac414) to head (5f5aa53).
Report is 3361 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4711   +/-   ##
=======================================
  Coverage   97.60%   97.60%           
=======================================
  Files          43       43           
  Lines        8922     8929    +7     
  Branches     1406     1406           
=======================================
+ Hits         8708     8715    +7     
  Misses         95       95           
  Partials      119      119           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.