Microsoft.data.sqlite#2163
Microsoft.data.sqlite#2163rwmcintosh merged 14 commits intoOpen-Systems-Pharmacology:v13from Felixmil:Microsoft.Data.Sqlite
Conversation
- Added a new Program.cs file for testing Microsoft.Data.Sqlite migration functionality. - Replaced System.Data.SQLite.Core with Microsoft.Data.Sqlite in multiple project files. - Updated connection handling in SessionFactoryProvider and ProjectFileCompressor to use Microsoft.Data.Sqlite. - Removed obsolete SQLite interop file handling from project files.
- Replace custom MicrosoftDataSqliteDriver with SQLite20Driver - SQLite20Driver is compatible with Microsoft.Data.Sqlite connection strings - Removes custom driver implementation that was causing build errors - Maintains NHibernate functionality while supporting Microsoft.Data.Sqlite - Resolves CI build error: MicrosoftDataSqliteDriver not found
- ProjectFileCompressor requires Microsoft.Data.Sqlite package - Fixes build error: CS0234 and CS0246 for SqliteConnection - Restores the package reference that was lost during git history cleanup
- Replace System.Data.SQLite.Core with Microsoft.Data.Sqlite in all projects - Update ProjectFileCompressor and NHibernate configuration for Microsoft.Data.Sqlite - Remove SQLite interop file handling (Microsoft.Data.Sqlite handles this automatically) Resolves macOS ARM64 P/Invoke stack overflow errors and provides cross-platform compatibility.
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the SQLite database provider from System.Data.SQLite.Core to Microsoft.Data.Sqlite version 8.0.0. This is a significant dependency change that modernizes the SQLite implementation across the MoBi application.
Key changes include:
- Replacing the legacy
System.Data.SQLite.Corepackage withMicrosoft.Data.Sqlitein all project files - Updating code references to use the new
SqliteConnectiontype fromMicrosoft.Data.Sqlite - Simplifying the SQLite connection string by removing legacy parameters (
Version=3,New=False,Compress=True) - Removing custom build targets and property groups that were specific to
System.Data.SQLite.Core
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MoBi.Tests/MoBi.Tests.csproj | Replaced System.Data.SQLite.Core package with Microsoft.Data.Sqlite and removed SQLite-specific build configuration |
| src/MoBi/MoBi.csproj | Updated package reference and removed SQLite-specific build targets |
| src/MoBi.Core/Services/ProjectFileCompressor.cs | Updated using statement and replaced SQLiteConnection with SqliteConnection |
| src/MoBi.Core/Serialization/ORM/SessionFactoryProvider.cs | Simplified connection string by removing legacy SQLite parameters |
| src/MoBi.Core/MoBi.Core.csproj | Added Microsoft.Data.Sqlite package reference |
| src/MoBi.BatchTool/MoBi.BatchTool.csproj | Updated package reference and removed SQLite-specific build configuration |
Comments suppressed due to low confidence (1)
src/MoBi.Core/Serialization/ORM/SessionFactoryProvider.cs:41
- The NHibernate driver
SQLite20Driveris designed for System.Data.SQLite. When migrating to Microsoft.Data.Sqlite, you may need to verify that this driver is compatible or consider using a different NHibernate driver that explicitly supports Microsoft.Data.Sqlite. Without proper driver compatibility, database operations through NHibernate may fail at runtime.
configuration.SetProperty("connection.driver_class", "NHibernate.Driver.SQLite20Driver");
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Clear connection pools when closing projects to release file handles. Fix test cleanup to properly close projects loaded by batch runner. Microsoft.Data.Sqlite enables connection pooling by default (unlike System.Data.SQLite), which keeps database files locked even after closing connections.
+ remove unrelated changes
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Exception occurs while opening the project! | ||
| // close the file and rethrow the exception | ||
| _sessionManager.CloseFactory(); | ||
| SqliteConnection.ClearAllPools(); |
There was a problem hiding this comment.
I need to find out why this is necessary here, but not in PK-Sim.
There was a problem hiding this comment.
According to System.Data.SQLite documentation the default pooling mode is off.
https://system.data.sqlite.org/home/doc/tip/System.Data.SQLite/SQLiteConnectionStringBuilder.cs
For Microsoft.Data.SQLite the default is on
I'm thinking that we should go back to OSPSuite.Core and change the connection strings to disable pooling and disable foreign key enforcement wherever the connection string appears
There was a problem hiding this comment.
I can remove this line (and others like it) with pooling disabled
There was a problem hiding this comment.
I'm thinking that we should go back to OSPSuite.Core and change the connection strings to disable pooling and disable foreign key enforcement wherever the connection string appears
Agree for the FK enforcement (and as for pooling: I just don't know enough about it for a qualified opinion).
There was a problem hiding this comment.
I'm thinking that we should go back to OSPSuite.Core and change the connection strings to disable pooling and disable foreign key enforcement wherever the connection string appears
Agree for the FK enforcement (and as for pooling: I just don't know enough about it for a qualified opinion).
Forgetting about whether or not pooling is good or not, to make it behave the way it behaved in V12 we just have to disable it explictly.
If we want to explore connection pooling, then we should do so in another project.
There was a problem hiding this comment.
Forgetting about whether or not pooling is good or not, to make it behave the way it behaved in V12 we just have to disable it explictly.
If we want to explore connection pooling, then we should do so in another project.
Fine with this.
rwmcintosh
requested review from
rwmcintosh
and removed request for
rwmcintosh
msevestre
left a comment
msevestre
left a comment
There was a problem hiding this comment.
We have no entry point here to do schema migration (what we have in PKSim). we will nee dot remember to do it
Closes #2164
Sister PRs:
Description
This PR migrates the MoBi project from
System.Data.SQLite.CoretoMicrosoft.Data.Sqliteto resolve critical compatibility issues on macOS ARM64. The migration provides full cross-platform support while maintaining all existing NHibernate functionality.Type of change
Please mark relevant options with an
xin the brackets.How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Reviewer checklist
Mark everything that needs to be checked before merging the PR.
This change requires a documentation updateabove is selectedScreenshots (if appropriate):
Questions (if appropriate):