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 what I think is a client bug #2383

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

erikbern
Copy link
Contributor

@erikbern erikbern commented Oct 23, 2024

This looks like a bug to me – closing any client resets the singleton.

Although the only impact is if you close clients then we'll recreate new clients unnecessarily. So fairly low impact.

@luiscape any memory of this? It's from PR #1030

@luiscape
Copy link
Member

@erikbern You may be right. I looked but can't find the details. @thundergolfer would you remember if this is needed?

Thank you for fixing -- sorry for the bug!

@erikbern
Copy link
Contributor Author

Hm I actually wonder if what we want to do is to remove the env client if that's the client that's being closed. I.e. something like this

if self is _Client._client_from_env:
    self.set_env_client(None)

Let me make that change!

@thundergolfer
Copy link
Contributor

Yep that makes sense @erikbern. Alternatively the memory_snapshot() function can just call set_env_client directly? I think it's the only consumer of this behavior.

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.

3 participants