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

Use singleton pattern to only initialize DB once #122

Merged
merged 3 commits into from
Sep 14, 2024
Merged

Conversation

dachengx
Copy link
Contributor

Similar to XENONnT/straxen#1426.

The motivation is to limit the number of connections to DB. With singleton pattern:

_instances: Dict[Tuple, "DB"] = {}
_initialized: Dict[Tuple, bool] = {}

def __new__(cls, *args, **kwargs):
    key = (args, frozenset(kwargs.items()))
    if key not in cls._instances:
        cls._instances[key] = super(DB, cls).__new__(cls)
        cls._initialized[key] = False
    return cls._instances[key]

def __init__(self, *args, **kwargs):
    key = (args, frozenset(kwargs.items()))
    if not self._initialized[key]:
        self._instances[key].initialize(*args, **kwargs)
        self._initialized[key] = True
    return

def initialize(self,

the instance will be only one given the same *args, **kwargs.

Also, this PR reorganizes logger by only retaining one handler, otherwise, you will see multiple messages with the same content.

@dachengx dachengx marked this pull request as ready for review September 13, 2024 13:57
@dachengx dachengx changed the title Use singleton pattern to only initialize DB once Use singleton pattern to only initialize DB once Sep 13, 2024
@yuema137
Copy link
Contributor

Hi, @dachengx. This is great! I have one concern regarding this update—with such an implementation, all different DB instances can only act like a global variable throughout the whole scope and lifetime of any script. I understand that this is usually the requirement for data processing, but I'm wondering if we are sure we have zero need to keep separate DBs? Probably also a question for @napoliion

@dachengx
Copy link
Contributor Author

@yuema137 I think you mean switching DB.

@dachengx
Copy link
Contributor Author

Sorry, let me first merge it for faster iteration. Please @yuema137 and @napoliion still comment about your opinion on this PR. If a problem happens I will make patches.

@dachengx dachengx merged commit e49fcba into master Sep 14, 2024
2 checks passed
@dachengx dachengx deleted the initialize_db_once branch September 14, 2024 23:45
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