bpo-38971: open file in codecs.open() closes if exception raised by caporta · Pull Request #17666 · python/cpython · GitHub
Skip to content

bpo-38971: open file in codecs.open() closes if exception raised - #17666

Merged
serhiy-storchaka merged 4 commits into
python:masterfrom
caporta:bpo-38971-fix-codecs-open
Mar 2, 2020
Merged

serhiy-storchaka merged 4 commits into
python:masterfrom
caporta:bpo-38971-fix-codecs-open

Conversation

@caporta

@caporta caporta commented Dec 19, 2019

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue38971

Open issue in the BPO indicated a desire to make the implementation of
codecs.open() at parity with io.open(), which implements a try/except to
assure file stream gets closed before an exception is raised.

https://bugs.python.org/issue38971

@the-knights-who-say-ni

Copy link
Copy Markdown

Open issue in the BPO indicated a desire to make the implementation of
codecs.open() at parity with io.open(), which implements a try/except to
assure file stream gets closed before an exception is raised.
@caporta
caporta force-pushed the bpo-38971-fix-codecs-open branch from 96e88dc to f6b7759 Compare December 19, 2019 22:45

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The fix LGTM, but I have some comments about the test.

Comment thread Lib/test/test_codecs.py Outdated
Comment thread Lib/test/test_codecs.py Outdated
Comment thread Lib/test/test_codecs.py Outdated
- Move spec out of seperate class and into CodecsModuleTest
- Pass invalid encoding to force exception rather than mock
- Specify that a LookupError is raised specifically
@caporta

caporta commented Dec 22, 2019

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka addressed the above comments, should be ready for re-review. thanks

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you. Yet a nitpick to tests.

Comment thread Lib/test/test_codecs.py Outdated
Prefer positional args to keywords in spec

Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>
@caporta
caporta requested review from serhiy-storchaka and removed request for serhiy-storchaka February 14, 2020 04:13
@caporta

caporta commented Feb 27, 2020

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka would it be possible to get a re-review of this PR? Thanks in advance!

@serhiy-storchaka
serhiy-storchaka merged commit 2565ede into python:master Mar 2, 2020
@serhiy-storchaka serhiy-storchaka added needs backport to 3.7 type-bug An unexpected behavior, bug, or error labels Mar 2, 2020
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @caporta for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @caporta for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot

Copy link
Copy Markdown

GH-18733 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 2, 2020
…thonGH-17666)

Open issue in the BPO indicated a desire to make the implementation of
codecs.open() at parity with io.open(), which implements a try/except to
assure file stream gets closed before an exception is raised.
(cherry picked from commit 2565ede)

Co-authored-by: Chris A <christopher.aporta@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 2, 2020
…thonGH-17666)

Open issue in the BPO indicated a desire to make the implementation of
codecs.open() at parity with io.open(), which implements a try/except to
assure file stream gets closed before an exception is raised.
(cherry picked from commit 2565ede)

Co-authored-by: Chris A <christopher.aporta@gmail.com>
@bedevere-bot

Copy link
Copy Markdown

miss-islington added a commit that referenced this pull request Mar 2, 2020
…-17666)

Open issue in the BPO indicated a desire to make the implementation of
codecs.open() at parity with io.open(), which implements a try/except to
assure file stream gets closed before an exception is raised.
(cherry picked from commit 2565ede)

Co-authored-by: Chris A <christopher.aporta@gmail.com>
miss-islington added a commit that referenced this pull request Mar 2, 2020
…-17666)

Open issue in the BPO indicated a desire to make the implementation of
codecs.open() at parity with io.open(), which implements a try/except to
assure file stream gets closed before an exception is raised.
(cherry picked from commit 2565ede)

Co-authored-by: Chris A <christopher.aporta@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants