Skip to content

Fix crash due to runtime import of typing_extensions#2507

Merged
A5rocks merged 1 commit intopython-trio:masterfrom
gschaffner:missing-typing_extensions-crash
Dec 25, 2022
Merged

Fix crash due to runtime import of typing_extensions#2507
A5rocks merged 1 commit intopython-trio:masterfrom
gschaffner:missing-typing_extensions-crash

Conversation

@gschaffner
Copy link
Member

hi!

#2502 added a runtime dependency on typing_extensions but did not update the packaging config to specify this dependency.

presently no runtime features of typing_extensions are being used, so it does not need to be a dependency at runtime.

(I have not included a changelog entry here since #2502 did not either and the bug this fixes was never user-facing.)

typing_extensions doesn't presently need to be a runtime dependency.
@codecov
Copy link

codecov bot commented Dec 20, 2022

Codecov Report

Merging #2507 (0da0068) into master (ec32bb8) will decrease coverage by 4.47%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2507      +/-   ##
==========================================
- Coverage   97.98%   93.51%   -4.48%     
==========================================
  Files         118      118              
  Lines       16327    16327              
  Branches     3156     3156              
==========================================
- Hits        15998    15268     -730     
- Misses        266      954     +688     
- Partials       63      105      +42     
Impacted Files Coverage Δ
trio/_unix_pipes.py 97.84% <100.00%> (-2.16%) ⬇️
trio/_windows_pipes.py 0.00% <0.00%> (-100.00%) ⬇️
trio/_wait_for_object.py 0.00% <0.00%> (-100.00%) ⬇️
trio/_core/_windows_cffi.py 0.00% <0.00%> (-100.00%) ⬇️
trio/_subprocess_platform/windows.py 0.00% <0.00%> (-100.00%) ⬇️
trio/_core/_io_windows.py 0.00% <0.00%> (-98.77%) ⬇️
trio/tests/test_wait_for_object.py 10.37% <0.00%> (-89.63%) ⬇️
trio/_core/tests/test_windows.py 17.82% <0.00%> (-82.18%) ⬇️
trio/tests/test_windows_pipes.py 26.58% <0.00%> (-73.42%) ⬇️
trio/_subprocess_platform/__init__.py 62.85% <0.00%> (-31.43%) ⬇️
... and 20 more

@gschaffner
Copy link
Member Author

the test suite timeout on CPython 3.9 on Windows x86-64 seems unrelated and likely just needs a rerun

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@A5rocks
Copy link
Contributor

A5rocks commented Dec 25, 2022

Looks good! Why didn't our test suite catch this? Do we need to separate out typing dependencies (mypy depends on typing_extensions) and test dependencies?

@A5rocks A5rocks merged commit 91a1350 into python-trio:master Dec 25, 2022
@gschaffner
Copy link
Member Author

yep, looks like it. see

trio/test-requirements.txt

Lines 124 to 127 in 4e7f21e

typing-extensions==4.4.0 ; implementation_name == "cpython"
# via
# -r test-requirements.in
# mypy

while in this case this bug would have been automatically caught if MyPy was not in the test dependencies, the same bug can always still go uncaught in the future if a package required for testing starts depending on typing_extensions.

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.

3 participants