search

aio-libs / aiohttp-sse

Server-sent events support for aiohttp
Other
194 stars 35 forks source link

Add type hints #441

Closed Olegt0rr closed 5 months ago

Olegt0rr commented 6 months ago

What do these changes do?

Added type hints

Are there changes in behavior for the user?

Related issue number

Closes #440

Checklist

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (a096742) 100.00% compared to head (83e1369) 100.00%. Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #441 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 4 4 Lines 439 468 +29 Branches 49 51 +2 ========================================= + Hits 439 468 +29 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Olegt0rr commented 6 months ago

@Dreamsorcerer, I don't know how to solve other typing errors

Olegt0rr commented 6 months ago

@Dreamsorcerer , there are some mypy issues still exists.

I don't know how to fix it. Should I add type: ignore[...] for them or we'll wait they become fixed by someone else?

Dreamsorcerer commented 6 months ago

I've been sorting out the aiohttp release, which is why I haven't done a proper review yet.

All of Cannot infer type argument 1 of "__setitem__" of "Application" errors are terrible error messages from mypy, which just mean the type you are assigning doesn't match the type of AppKey. You can use reveal_type() to figure out what type mypy is actually inferring.

There are some missing return types, which can just be set to Any if you've got no idea (and further errors ignored).

I'll go over it properly soon.

Olegt0rr commented 6 months ago

Weird behavior

Coverage: python 3.8: 100% python 3.9: 100% python 3.10: 100% python3.11: < 100% python 3.12: 100%

Report: htmlcov.zip Coverage fails on every return from .wait() But all tests still passes 🤨

P.S.: this problem is not related to this PR, cause reproduced in master, but since covdefaults has fail_under = 100 setting - it fail tests for this PR

@Dreamsorcerer , it seems I should turn settings back to fail_under = 0 for this PR and then open an issue for coverage. Is it right?

Dreamsorcerer commented 6 months ago

but since covdefaults has fail_under = 100 setting - it fail tests for this PR

Ah, I didn't realise covdefaults adds that as well. Maybe it's easier to go back too what you had before then? I don't mind too much.

Dreamsorcerer commented 6 months ago

I think reason might be a difference in cancellation behaviour on 3.11 maybe. So, the handler gets cancelled when the client disconnects, and some subtle difference makes this happen on 3.11 before returning.

Dreamsorcerer commented 6 months ago

I think this is almost ready.

Olegt0rr commented 6 months ago

but since covdefaults has fail_under = 100 setting - it fail tests for this PR

Ah, I didn't realise covdefaults adds that as well. Maybe it's easier to go back too what you had before then? I don't mind too much.

Okay! Removed covdefaults, but copied exclude_lines from their cfg :)

Olegt0rr commented 5 months ago

@Dreamsorcerer , Thank you for your patient review))