Skip to content

gh-100585: Fixed a bug where importlib.resources.as_file was leaving file pointers open#100586

Merged
jaraco merged 2 commits intopython:mainfrom
samety:fix-issue-100585
Dec 28, 2022
Merged

gh-100585: Fixed a bug where importlib.resources.as_file was leaving file pointers open#100586
jaraco merged 2 commits intopython:mainfrom
samety:fix-issue-100585

Conversation

@samety
Copy link
Copy Markdown
Contributor

@samety samety commented Dec 28, 2022

gh-100585: Fixed a bug where importlib.resources.as_file was leaving file pointers open

importlib.resources.as_file was leaving temporary file pointers open in the _write_contents function.
Fixed it by using Path.write_bytes instead of open('wb').write(...)

@samety samety changed the title Fix issue 100585 gh-100585: Fixed a bug where importlib.resources.as_file was leaving file pointers open Dec 28, 2022
@bedevere-bot bedevere-bot mentioned this pull request Dec 28, 2022
Closed
@FFY00
Copy link
Copy Markdown
Member

FFY00 commented Dec 28, 2022 •
edited
Loading

@samety thanks for the fix! It does look correct to me.

@jaraco, we already have a fix in https://github.com/python/importlib_resources (python/importlib_resources@a4b3ef8), how do you want to handle these situations?

@jaraco
Copy link
Copy Markdown
Member

jaraco commented Dec 28, 2022

I have plans to merge the fix from _resources to here, but I think I'd prefer to get the write_bytes technique there first. Let's do that and then I'll plan to merge them here. I'll plan to cherry-pick the change from here into importlib_resources.

@jaraco
Copy link
Copy Markdown
Member

jaraco commented Dec 28, 2022

On further consideration, it's probably fine to merge this now. Since importlib_resources v5.10.2 applies the same change, It won't conflict when importlib_resources is merged.

@jaraco
Copy link
Copy Markdown
Member

jaraco commented Dec 28, 2022

I looked at the failing test and it seems to be unrelated.

@jaraco jaraco merged commit f10f503 into python:main Dec 28, 2022
@samety samety deleted the fix-issue-100585 branch December 28, 2022 22:28
@jaraco jaraco added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Dec 29, 2022
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Dec 29, 2022

Thanks @samety for the PR, and @jaraco for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Dec 29, 2022

Thanks @samety for the PR, and @jaraco for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Dec 29, 2022

Sorry, @samety and @jaraco, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker f10f503b24a35a43910a632ee50c7568bedd6664 3.11

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Dec 29, 2022

Sorry @samety and @jaraco, I had trouble checking out the 3.10 backport branch.
Please retry by removing and re-adding the "needs backport to 3.10" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker f10f503b24a35a43910a632ee50c7568bedd6664 3.10

@jaraco jaraco removed needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Dec 29, 2022
jaraco pushed a commit to jaraco/cpython that referenced this pull request Feb 17, 2023
@samety @jaraco
Cherry-pick write_bytes technique from pythongh-100586.
d623078
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaraco jaraco Awaiting requested review from jaraco jaraco is a code owner

@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

@brettcannon brettcannon Awaiting requested review from brettcannon

@FFY00 FFY00 Awaiting requested review from FFY00 FFY00 is a code owner

Assignees

@jaraco jaraco

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@samety @FFY00 @jaraco @miss-islington @bedevere-bot