Skip to content

Conversation

@elemoine
Copy link
Contributor

Overview

Do not call Session.close_all as this will close all the Sessions that exist in memory, including the sessions that are specific to the application and unrelated to PyPWS.

And here the closing of sessions is already handled by _LAST_SESSION.close(), so we can just remove the call to _SESSION_MAKER.close_all.

Related Issue / Discussion

None

Additional Information

None. The overview covers it all.

Contribution Agreement

(as per https://github.com/geopython/pywps/blob/master/CONTRIBUTING.rst#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to PyWPS. I confirm that my contributions to PyWPS will be compatible with the PyWPS license guidelines at the time of contribution.
  • I have already previously agreed to the PyWPS Contributions and Licensing Guidelines

Do not call Session.close_all as this will close all the Sessions
that exist in memory, including the sessions that are specific to
the application and unrelated to PyPWS.

And here the closing of sessions is already handled by
_LAST_SESSION.close(), so we can just remove the call to
_SESSION_MAKER.close_all.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.009%) to 74.881% when pulling 25161b3 on elemoine:ele_close-all into ec1d90f on geopython:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.009%) to 74.881% when pulling 25161b3 on elemoine:ele_close-all into ec1d90f on geopython:master.

@cehbrecht cehbrecht added this to the 4.4.0 milestone Dec 20, 2018
@cehbrecht cehbrecht added the bug label Dec 20, 2018
Copy link
Collaborator

@cehbrecht cehbrecht left a comment

Choose a reason for hiding this comment

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

Sounds persuasive.

@elemoine
Copy link
Contributor Author

In other words a library should never call Session.close_all, as this may close sessions belonging to the application, or to other libraries used by the application.

@cehbrecht cehbrecht merged commit e10f641 into geopython:master Dec 21, 2018
@cehbrecht
Copy link
Collaborator

@elemoine done. Thanks :)

@elemoine elemoine deleted the ele_close-all branch December 21, 2018 13:21
@elemoine
Copy link
Contributor Author

Thanks to you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants