bpo-33468: Add try-finally contextlib.contextmanager example #7816

Carreau · 2018-06-20T08:06:23Z

This increase the chances of people following this example properly
cleaning up resources.

https://bugs.python.org/issue33468

taleinat

Please change to use an entirely abstract example according to @ncoghlan's suggestion on bpo-33468, or close this PR.

bedevere-bot · 2018-06-30T13:41:18Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Carreau · 2018-07-23T17:59:07Z

I have made the requested changes; please review again

bedevere-bot · 2018-07-23T17:59:10Z

Thanks for making the requested changes!

@taleinat: please review the changes made to this pull request.

taleinat

Looking good, a few minor changes needed IMO.

taleinat · 2018-07-23T18:18:41Z

Doc/library/contextlib.rst

+   and doesn't implement a ``close()`` method for use with ``contextlib.closing``
+
+   A simple example thus would be the following to ensure correct resource
+   management::


IMO the fact that this an abstract example (rather than a concrete one) should be mentioned explicitly.

Also, I suggest removing the word "simple".

taleinat · 2018-07-23T18:21:17Z

Doc/library/contextlib.rst

-      <h1>
-      foo
-      </h1>
+      >>> with managed_resource(*args, **kwds) as resource:


Perhaps avoid *args, **kwds here? Instead consider e.g. with managed_resource(foo="bar") as resource:

something more explicit ? timeout=3600 ?

bedevere-bot · 2018-07-23T18:23:45Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

This increase the chances of people following this example properly cleaning up resources.

Carreau · 2018-07-23T19:02:49Z

I have made the requested changes; please review again

bedevere-bot · 2018-07-23T19:02:51Z

Thanks for making the requested changes!

@taleinat: please review the changes made to this pull request.

miss-islington · 2018-07-23T21:10:58Z

Thanks @Carreau for the PR, and @taleinat for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6, 3.7.
🐍🍒⛏🤖

…H-7816) (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

bedevere-bot · 2018-07-23T21:11:06Z

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

…H-7816) (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

Carreau · 2018-07-23T21:12:07Z

Thanks @taleinat for the review and the merge !

bedevere-bot · 2018-07-23T21:12:08Z

GH-8426 is a backport of this pull request to the 3.6 branch.

miss-islington · 2018-07-23T21:13:01Z

Sorry, @Carreau and @taleinat, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker bde782bb594edffeabe978abeee2b7082ab9bc2a 2.7

…ythonGH-7816). (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

bedevere-bot · 2018-07-23T21:18:21Z

GH-8427 is a backport of this pull request to the 2.7 branch.

GH-8425) (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

GH-8426) (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

…H-7816) (GH-8427) (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

Carreau requested a review from 1st1 as a code owner June 20, 2018 08:06

the-knights-who-say-ni added the CLA signed label Jun 20, 2018

bedevere-bot added the awaiting review label Jun 20, 2018

taleinat requested changes Jun 30, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Jun 30, 2018

Carreau force-pushed the bpo-33468 branch from f0a1657 to d35a22a Compare July 23, 2018 17:58

bedevere-bot added awaiting change review and removed awaiting changes labels Jul 23, 2018

taleinat requested changes Jul 23, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels Jul 23, 2018

bpo-33468: Add try-finally contextlib.contextmanager example

9ab8fca

This increase the chances of people following this example properly cleaning up resources.

Carreau force-pushed the bpo-33468 branch from d35a22a to 9ab8fca Compare July 23, 2018 19:02

bedevere-bot added awaiting change review and removed awaiting changes labels Jul 23, 2018

contextmanager docs wording improvement

d29322b

taleinat approved these changes Jul 23, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Jul 23, 2018

taleinat added skip news needs backport to 3.6 docs Documentation in the Doc dir labels Jul 23, 2018

taleinat added the needs backport to 3.7 label Jul 23, 2018

taleinat merged commit bde782b into python:master Jul 23, 2018

bedevere-bot removed the awaiting merge label Jul 23, 2018

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 23, 2018

bpo-33468: Add try-finally contextlib.contextmanager example (pythonG…

d5e5316

…H-7816) (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

bedevere-bot removed the needs backport to 3.7 label Jul 23, 2018

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 23, 2018

bpo-33468: Add try-finally contextlib.contextmanager example (pythonG…

3465cb5

…H-7816) (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

bedevere-bot removed the needs backport to 3.6 label Jul 23, 2018

miss-islington assigned taleinat Jul 23, 2018

taleinat pushed a commit to taleinat/cpython that referenced this pull request Jul 23, 2018

[2.7] bpo-33468: Add try-finally contextlib.contextmanager example (p…

947efaf

…ythonGH-7816). (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

bedevere-bot removed the needs backport to 2.7 label Jul 23, 2018

taleinat pushed a commit that referenced this pull request Jul 23, 2018

bpo-33468: Add try-finally contextlib.contextmanager example (GH-7816) (

4e166ff

GH-8425) (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

taleinat pushed a commit that referenced this pull request Jul 23, 2018

bpo-33468: Add try-finally contextlib.contextmanager example (GH-7816) (

5b3643d

GH-8426) (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

taleinat added a commit that referenced this pull request Jul 23, 2018

[2.7] bpo-33468: Add try-finally contextlib.contextmanager example (G…

f7e60a6

…H-7816) (GH-8427) (cherry picked from commit bde782b) Co-authored-by: Matthias Bussonnier <[email protected]>

Uh oh!

bpo-33468: Add try-finally contextlib.contextmanager example #7816

bpo-33468: Add try-finally contextlib.contextmanager example #7816

Uh oh!

Conversation

Carreau commented Jun 20, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taleinat left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jun 30, 2018

Uh oh!

Carreau commented Jul 23, 2018

Uh oh!

bedevere-bot commented Jul 23, 2018

Uh oh!

taleinat left a comment

Choose a reason for hiding this comment

Uh oh!

taleinat Jul 23, 2018

Choose a reason for hiding this comment

Uh oh!

taleinat Jul 23, 2018

Choose a reason for hiding this comment

Uh oh!

Carreau Jul 23, 2018

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jul 23, 2018

Uh oh!

Carreau commented Jul 23, 2018

Uh oh!

bedevere-bot commented Jul 23, 2018

Uh oh!

miss-islington commented Jul 23, 2018

Uh oh!

bedevere-bot commented Jul 23, 2018

Uh oh!

Carreau commented Jul 23, 2018

Uh oh!

bedevere-bot commented Jul 23, 2018

Uh oh!

miss-islington commented Jul 23, 2018

Uh oh!

bedevere-bot commented Jul 23, 2018

Uh oh!

Uh oh!

Carreau commented Jun 20, 2018 •

edited by bedevere-bot

Loading

taleinat left a comment •

edited

Loading