Skip to content
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

Session started at Line 169 is closed after the use #1378

Closed
wants to merge 1 commit into from

Conversation

johirbuet
Copy link

Session started at line 169 is not closed after use.
This PR closes that session after its usage.

@nfelt
Copy link
Contributor

nfelt commented Aug 24, 2018

Sessions close themselves upon deletion/garbage collection, so after it's out of scope it should eventually get cleaned up, I would have thought. Was this fixing some specific issue you had?

Also, the code under plugins/hparams is stale code - it will probably be replaced completely at some future date.

@johirbuet
Copy link
Author

johirbuet commented Aug 24, 2018

I was looking at this comment and thought it might be good practise to close the session

@nfelt
Copy link
Contributor

nfelt commented Aug 24, 2018

I mean, it doesn't hurt to close it but it doesn't seem particularly necessary to me given that the resources will be cleaned up anyway, and this is only demo code.

@nfelt
Copy link
Contributor

nfelt commented Sep 20, 2018

Closing due to inactivity and since I'm not sure closing the session explicitly is an improvement.

@nfelt nfelt closed this Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants