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

[PR #6923/78fa0401 backport][3.9] Exclude the object possibility from the ClientSession.timeout type #6924

Conversation

patchback[bot]
Copy link
Contributor

@patchback patchback bot commented Sep 20, 2022

This is a backport of PR #6923 as merged into master (78fa040).

What do these changes do?

This is a follow-up PR to #6917, which fixes the overly inclusive typing of ClientSession.timeout attribute, which can never actually end up as an object instance. This leftover probably exists from the time when there was the _sentinel value being assigned to the attribute (see "3.8 branch self._timeout assignment" from the linked issue), hence why it ended up this way.

Are there changes in behavior for the user?

For untyped code, there's absolutely no change - the self._timeout attribute was never actually assigned the sentinel value to. Most of the explanation already exists in #6917, here's the test code snippet from there again:

import asyncio
import aiohttp

async def main:
    session = aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=10))
    # read back the total time attribute
    total_time = session.timeout.total  # "object" type of "Union[object, ClientTimeout]" has no attribute "total"
    print(total_time)

asyncio.run(main())

For typed code, the current solution would be to assert isinstance(session.timeout, aiohttp.ClientTimeout) everywhere the attribute is being accessed. This PR removes this "unnecessary necessity".

Related issue number

#6917

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes (documentation never mentioned an object instance being accessible from there even)
  • 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."

This is a follow-up PR to #6917, which fixes the overly inclusive typing
of `ClientSession.timeout` attribute, which can never actually end up as
an `object` instance. This leftover probably exists from the time when
there was the `_sentinel` value being assigned to the attribute (see
"3.8 branch `self._timeout` assignment" from the linked issue), hence
why it ended up this way.

For untyped code, there's absolutely no change — the `self._timeout`
attribute was never actually assigned the sentinel value to. Most of the
explanation already exists in #6917, here's the test code snippet from
there again:

```py
import asyncio
import aiohttp

async def main:
    session = aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=10))
    # read back the total time attribute
    total_time = session.timeout.total  # "object" type of "Union[object, ClientTimeout]" has no attribute "total"
    print(total_time)

asyncio.run(main())
```

For typed code, the current solution would be to `assert
isinstance(session.timeout, aiohttp.ClientTimeout)` everywhere the
attribute is being accessed. This PR removes this "unnecessary
necessity".

Co-authored-by: Sviatoslav Sydorenko <wk@sydorenko.org.ua>

Fixes #6917
PR #6923

(cherry picked from commit 78fa040)
@patchback patchback bot requested a review from asvetlov as a code owner September 20, 2022 16:49
@webknjaz webknjaz merged commit 03f4dd0 into 3.9 Sep 20, 2022
@webknjaz webknjaz deleted the patchback/backports/3.9/78fa04019c49e87f352d89e000963c16af78cae1/pr-6923 branch September 20, 2022 16:52
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