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

Rework to allow use of metadata #13

Merged
merged 30 commits into from
Jul 3, 2024

Conversation

MatBarba
Copy link
Collaborator

@MatBarba MatBarba commented Jun 28, 2024

Maybe too many things changed, but I needed those to ensure I can use a metadata with :

  • DBConnection can now be created without a schema (dump_dir no longer mandatory)
  • DBConnection accepts an Metadata object instead of relying on an SQL file and a reflection
  • Add 2 methods to DBConnection: create_all_tables and create_table (requires the metadata to be set as above)
  • If both a dump_dir and a metadata are provided to a UnitTestDB, the schema is made from the metadata and the data is loaded from the dump_dir
  • Add a context manager to properly dispose of the UnitTestDB (UnitTestDBContext)

Tests:

  • Add a simple mock base and mock_metadata
  • When possible, use a context session_scope or test_session_scope to avoid hanging connections (unless we are specifically testing for that)
  • Both SQLalchemy and MySQL should work
  • Add an argument tmp_path to the UnitTestDB when asked to create an SQLite database test file

Usage:
Say you want to test interactions with a whole schema described in a given metadata. You can do so with:

with UnitTestDBContext(server_url, metadata=metadata, tmp_path=tmp_path, name="test_metadata") as test_db:
    with test_db.dbc.test_session_scope() as session:
        results = session.execute(text("SELECT * FROM foo"))

The first context ensures the database is properly disposed of no matter what, and the second ensures the changes are rolledback and the connection returned.

Copy link
Collaborator

@JAlvarezJarreta JAlvarezJarreta left a comment

Choose a reason for hiding this comment

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

Quite a few interesting changes indeed. I quite like the approach and functionality proposed, great addition 👍

I have added a few suggestions, changes and questions for discussion. I believe the 2 new test methods in test_unittestdb.py could be merged into one, as it feels the functionality and objectives are very similar.

pyproject.toml Outdated Show resolved Hide resolved
src/ensembl/utils/database/dbconnection.py Outdated Show resolved Hide resolved
src/ensembl/utils/database/dbconnection.py Outdated Show resolved Hide resolved
tests/database/test_dbconnection.py Outdated Show resolved Hide resolved
tests/database/test_dbconnection.py Show resolved Hide resolved
tests/database/test_unittestdb.py Outdated Show resolved Hide resolved
tests/database/test_unittestdb.py Outdated Show resolved Hide resolved
tests/database/test_unittestdb.py Outdated Show resolved Hide resolved
tests/database/test_unittestdb.py Outdated Show resolved Hide resolved
tests/database/test_unittestdb.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
src/ensembl/utils/database/dbconnection.py Outdated Show resolved Hide resolved
src/ensembl/utils/database/dbconnection.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
tests/database/test_unittestdb.py Outdated Show resolved Hide resolved
src/ensembl/utils/database/unittestdb.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@JAlvarezJarreta JAlvarezJarreta left a comment

Choose a reason for hiding this comment

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

We got there at the end!

@JAlvarezJarreta JAlvarezJarreta merged commit 6348981 into Ensembl:main Jul 3, 2024
5 checks passed
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.

None yet

2 participants