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

fix session._connection none error #250

Merged
merged 2 commits into from
Dec 12, 2022

Conversation

liuxiaocs7
Copy link
Contributor

close #249

wey-gu
wey-gu previously approved these changes Dec 11, 2022
@wey-gu wey-gu requested a review from Aiee December 11, 2022 17:36
@codecov-commenter
Copy link

codecov-commenter commented Dec 11, 2022

Codecov Report

Base: 77.48% // Head: 77.11% // Decreases project coverage by -0.36% ⚠️

Coverage data is based on head (9bb2de3) compared to base (f028dfc).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #250      +/-   ##
==========================================
- Coverage   77.48%   77.11%   -0.37%     
==========================================
  Files          18       18              
  Lines        2407     2408       +1     
==========================================
- Hits         1865     1857       -8     
- Misses        542      551       +9     
Impacted Files Coverage Δ
nebula3/gclient/net/SessionPool.py 71.61% <0.00%> (-0.31%) ⬇️
nebula3/mclient/__init__.py 69.23% <0.00%> (-2.68%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Aiee
Copy link
Contributor

Aiee commented Dec 12, 2022

Hello @liuxiaocs7

Thank you for bringing this to us.

When we release a session here we want to make sure two things: 1. the session is logged out 2. the connection is closed. The fix you proposed here didn't deal with the connection. You could just save the connection to a variable before releasing the session and close the connection later. Something like :

conn = session._connection
session.release()
conn.close()

@liuxiaocs7
Copy link
Contributor Author

Hello @liuxiaocs7

Thank you for bringing this to us.

When we release a session here we want to make sure two things: 1. the session is logged out 2. the connection is closed. The fix you proposed here didn't deal with the connection. You could just save the connection to a variable before releasing the session and close the connection later. Something like :

conn = session._connection
session.release()
conn.close()

Thanks for your kindly reply, got it, i'll fix

Copy link
Contributor

@Aiee Aiee left a comment

Choose a reason for hiding this comment

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

LGTM.

@Aiee Aiee merged commit 74db805 into vesoft-inc:master Dec 12, 2022
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.

connection is None in _remove_idle_unusable_session after session.close()
4 participants