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

Refactor database handler initialization with factory function #4

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

Conversation

MPoppinga
Copy link
Owner

  • Added get_db_handler() factory function in db_handler/__init__.py
  • Updated add_to_cache.py and monitor_cache_queue.py to use new factory function
  • Simplified database handler creation and import logic
  • Improved error handling for unsupported database backends

- Added `get_db_handler()` factory function in `db_handler/__init__.py`
- Updated `add_to_cache.py` and `monitor_cache_queue.py` to use new factory function
- Simplified database handler creation and import logic
- Improved error handling for unsupported database backends
@MPoppinga MPoppinga requested a review from Copilot March 3, 2025 21:15

Choose a reason for hiding this comment

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

PR Overview

This PR refactors database handler initialization by introducing a factory function to simplify and unify the creation of database handlers. The changes include replacing direct instantiation with the new get_db_handler factory, updating error handling for unsupported backends, and streamlining import logic.

Reviewed Changes

File Description
src/partitioncache/cli/add_to_cache.py Uses get_db_handler instead of direct class instantiation; removes a db_env_file check.
src/partitioncache/cli/monitor_cache_queue.py Updates database handler instantiation to use get_db_handler while retaining an environment check.
src/partitioncache/db_handler/init.py Introduces the get_db_handler factory function for handling different database types.

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/partitioncache/cli/add_to_cache.py:68

  • [nitpick] Consider aligning the backend naming conventions by using 'postgres' consistently in both the condition and the factory call, to improve clarity and reduce potential confusion.
if args.db_backend == "postgresql":

src/partitioncache/cli/add_to_cache.py:66

  • The removal of the db_env_file check in the postgres branch here contrasts with its enforcement in monitor_cache_queue; consider enforcing this check uniformly if the db_env_file input is required for proper initialization.
if args.db_env_file is None:
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.

1 participant