Skip to content

gh-131233: remove return-in-finally in multiprocessing/connection.py - #131416

Merged
iritkatriel merged 3 commits into
python:mainfrom
iritkatriel:gh-131233
Mar 21, 2025
Merged

iritkatriel merged 3 commits into
python:mainfrom
iritkatriel:gh-131233

Conversation

@iritkatriel

@iritkatriel iritkatriel commented Mar 18, 2025 •

Copy link
Copy Markdown
Member

@markshannon

Copy link
Copy Markdown
Member

I'm struggling to see any difference in behavior here, so:

  • I'm not understanding this, or
  • the original code wasn't buggy (I think that is the case), or
  • this doesn't fix the bug.

Or is this just to fix the warning and not change the semantics?

@encukou

encukou commented Mar 21, 2025

Copy link
Copy Markdown
Member

The warning causes a flaky test, see https://buildbot.python.org/#/builders/29/builds/7932

Comment thread Lib/multiprocessing/connection.py Outdated
Comment thread Lib/multiprocessing/connection.py Outdated
@nascheme

Copy link
Copy Markdown
Member

I agree the "is" and "is not" would be slightly better then ==. Also, I think it helps to "push down" the code that's inside the outer try/except into a helper method. The triple nested try/except is quite hard to understand.

Comment thread Lib/multiprocessing/connection.py Outdated
Comment thread Lib/multiprocessing/connection.py Outdated
@iritkatriel

Copy link
Copy Markdown
Member Author

I agree the "is" and "is not" would be slightly better then ==. Also, I think it helps to "push down" the code that's inside the outer try/except into a helper method. The triple nested try/except is quite hard to understand.

This can be done as a further improvement. In this PR I'd rather make the change more straightforward to see from the diff.

@iritkatriel
iritkatriel enabled auto-merge (squash) March 21, 2025 17:42
@markshannon

Copy link
Copy Markdown
Member

The warning causes a flaky test, see https://buildbot.python.org/#/builders/29/builds/7932

The issue says there is a bug, not just a warning. What's the bug?

@iritkatriel

Copy link
Copy Markdown
Member Author

The warning causes a flaky test, see https://buildbot.python.org/#/builders/29/builds/7932

The issue says there is a bug, not just a warning. What's the bug?

See discussion on the issue - assumption now is that it's not a bug. I'll fix the title.

@iritkatriel
iritkatriel merged commit 3e2ccea into python:main Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants