New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ResourceWarning warnings #25

Merged
merged 1 commit into from Dec 7, 2016

Conversation

Projects
None yet
3 participants
@vstinner
Contributor

vstinner commented Jun 29, 2016

  • RepositoryFactory.open(): close the stream after reading its
    content
  • Repository._next_stream(): use "with" to close the next-stream file
@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Jun 29, 2016

Contributor

Hum, it looks like Travis still runs tests on Python 3.2, but empty_fill = u'∙' in pip/_vendor/progress/bar.py raises a SyntaxError. It's time to stop running tests on Python 3.2, no?

Contributor

vstinner commented Jun 29, 2016

Hum, it looks like Travis still runs tests on Python 3.2, but empty_fill = u'∙' in pip/_vendor/progress/bar.py raises a SyntaxError. It's time to stop running tests on Python 3.2, no?

@mtreinish

This comment has been minimized.

Show comment
Hide comment
@mtreinish

mtreinish Jul 1, 2016

Contributor

I pushed: #28 to remove py3.2 testing from travis

Contributor

mtreinish commented Jul 1, 2016

I pushed: #28 to remove py3.2 testing from travis

Fix a ResourceWarning warnings
Use context managers to ensure that file are always closed.

@vstinner vstinner changed the title from Fix two ResourceWarning warnings to Fix ResourceWarning warnings Nov 29, 2016

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Nov 29, 2016

Contributor

The change to remove Python 3.2 from Travis has been removed. I rebased my change.

Contributor

vstinner commented Nov 29, 2016

The change to remove Python 3.2 from Travis has been removed. I rebased my change.

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Nov 29, 2016

Contributor

@rbtcollins & @freeekanayaka: would you mind to review this straighforward change? Travis CI is now happy (before, it only failed because of Python 3.2).

Contributor

vstinner commented Nov 29, 2016

@rbtcollins & @freeekanayaka: would you mind to review this straighforward change? Travis CI is now happy (before, it only failed because of Python 3.2).

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Nov 30, 2016

Contributor

FYI this pull request doesn't fix all ResourceWarning warnings, but first the most obvious warnings. It helps to debug the remaining ones ;-)

Contributor

vstinner commented Nov 30, 2016

FYI this pull request doesn't fix all ResourceWarning warnings, but first the most obvious warnings. It helps to debug the remaining ones ;-)

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Dec 5, 2016

Contributor

Ping @freeekanayaka ? Can you please review this pull request?

Contributor

vstinner commented Dec 5, 2016

Ping @freeekanayaka ? Can you please review this pull request?

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Dec 6, 2016

Contributor

Ping?

Contributor

vstinner commented Dec 6, 2016

Ping?

@thomir

This comment has been minimized.

Show comment
Hide comment
@thomir

thomir Dec 7, 2016

Member

LGTM

Member

thomir commented Dec 7, 2016

LGTM

@thomir thomir merged commit 4eff0df into testing-cabal:master Dec 7, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Dec 7, 2016

Contributor

Thank you!

I only fixed the most obvious issues, there are still a few remaining ResourceWarning, more tricky to fix. But it should now be easier to catch them.

Contributor

vstinner commented Dec 7, 2016

Thank you!

I only fixed the most obvious issues, there are still a few remaining ResourceWarning, more tricky to fix. But it should now be easier to catch them.

@vstinner vstinner deleted the vstinner:res_warn branch Dec 7, 2016

mtreinish added a commit to mtreinish/stestr that referenced this pull request Aug 9, 2017

Ensure we always close files in file repository
In newer versions of python we get a warning if we don't explictly close
an open file. This commit address those warnings by ensuring we always
close any open files by using a context manager.

This was ported from the testrepository patch (which landed soon after
stestr was forked):

testing-cabal/testrepository#25

mtreinish added a commit to mtreinish/stestr that referenced this pull request Aug 9, 2017

Ensure we always close files in file repository
In newer versions of python we get a warning if we don't explictly close
an open file. This commit address those warnings by ensuring we always
close any open files by using a context manager.

This was ported from the testrepository patch (which landed soon after
stestr was forked):

testing-cabal/testrepository#25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment