colcon / colcon-parallel-executor

Extension for colcon to process packages in parallel
http://colcon.readthedocs.io
Apache License 2.0
2 stars 4 forks source link

Drop Windows event loop hack in Python 3.8+ #34

Closed cottsay closed 1 year ago

cottsay commented 1 year ago

In Python versions prior to 3.8, it appears that attempting to close the event loop after a Ctrl-C would reliably hang the process, which would need to be killed.

I've been unable to reproduce the behavior in any newer Python versions, so I think it's time to set the timeline for removing the hack entirely.

This should take care of the common ResourceWarning messages when using newer Python versions.

This change was already made in colcon/colcon-core#573 and colcon/colcon-core#581.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.14% :tada:

Comparison is base (32ef9ac) 80.91% compared to head (c8162ad) 81.06%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #34 +/- ## ========================================== + Coverage 80.91% 81.06% +0.14% ========================================== Files 2 2 Lines 131 132 +1 Branches 40 41 +1 ========================================== + Hits 106 107 +1 Misses 16 16 Partials 9 9 ``` | [Files Changed](https://app.codecov.io/gh/colcon/colcon-parallel-executor/pull/34?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=colcon) | Coverage Δ | | |---|---|---| | [colcon\_parallel\_executor/executor/parallel.py](https://app.codecov.io/gh/colcon/colcon-parallel-executor/pull/34?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=colcon#diff-Y29sY29uX3BhcmFsbGVsX2V4ZWN1dG9yL2V4ZWN1dG9yL3BhcmFsbGVsLnB5) | `80.91% <100.00%> (+0.14%)` | :arrow_up: |

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

cottsay commented 1 year ago

As far as I understand, this is kind of a bugfix for Python < 3.8 versions, right?

I'd call it a bugfix for Python >= 3.8 actually. Earlier than that, we still can't reliably call close() on the loop without hanging the process, but in Python >= 3.8, we seem to be able to do so and it also started producing a ResourceWarning letting us know that we forgot to close the loop.

There should be no change in behavior for Python < 3.8.