Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 22, 2025

The LMFDBDatabase constructor now accepts an optional config parameter, enabling flexible database configuration without requiring a full Configuration object.

Changes

  • Added optional config parameter to LMFDBDatabase.__init__()
  • Supports three input types:
    • None (default): Creates standard Configuration() object (preserves existing behavior)
    • dict: Dictionary with optional keys postgresql_options, flask_options, logging_options
    • Configuration object: Used directly without modification

Usage Examples

# Default behavior (unchanged)
db = LMFDBDatabase()

# Custom database connection
config = {
    'postgresql_options': {
        'host': 'custom-server.example.com',
        'dbname': 'my_lmfdb_instance',
        'user': 'researcher'
    },
    'logging_options': {
        'editor': 'researcher_name'
    }
}
db = LMFDBDatabase(config=config)

# Minimal override (just change editor)
db = LMFDBDatabase(config={'logging_options': {'editor': 'script_user'}})

Implementation Details

When a config dictionary is provided, the constructor creates a ConfigWrapper class that provides the same interface as the Configuration class, ensuring compatibility with the underlying PostgresDatabase parent class.

The change maintains full backward compatibility - all existing code using LMFDBDatabase() continues to work without modification.

Fixes #6652.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: edgarcosta <322822+edgarcosta@users.noreply.github.com>
Copilot AI changed the title [WIP] One should be able to pass a config dictionary to LMFDBDatabase Allow passing a config dictionary to LMFDBDatabase constructor Aug 22, 2025
Copilot AI requested a review from edgarcosta August 22, 2025 17:38
@edgarcosta
Copy link
Member

@copilot Can we factor out ConfigWrapper ?

Co-authored-by: edgarcosta <322822+edgarcosta@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 22, 2025

@copilot Can we factor out ConfigWrapper ?

Done! I've factored out the ConfigWrapper class from the LMFDBDatabase.__init__ method and moved it to module level. This improves code organization, makes it reusable, and makes the __init__ method cleaner and easier to read.

The functionality remains identical - all tests pass and the refactoring maintains full backward compatibility.

Changes in commit a7b2096.

@edgarcosta
Copy link
Member

@copilot Great! now move the ConfigWrapper to lmfdb/utils/config.py

Co-authored-by: edgarcosta <322822+edgarcosta@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 22, 2025

@copilot Great! now move the ConfigWrapper to lmfdb/utils/config.py

Done! I've moved the ConfigWrapper class to lmfdb/utils/config.py as requested. The class is now properly organized in the utils module and imported in lmfdb_database.py. All functionality remains identical.

Changes in commit 16e52ad.

@edgarcosta
Copy link
Member

@havarddj would this make your life easier?

@edgarcosta edgarcosta marked this pull request as ready for review October 7, 2025 21:25
@edgarcosta edgarcosta merged commit f00aa8e into main Oct 7, 2025
46 checks passed
@roed314 roed314 deleted the copilot/fix-6652 branch October 10, 2025 18:08
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.

One should be able to pass a config dictionary to LMFDBDatabase

2 participants