Microsoft.data.sqlite#3323
Microsoft.data.sqlite#3323rwmcintosh merged 26 commits intoOpen-Systems-Pharmacology:V13from Felixmil:Microsoft.Data.Sqlite
Conversation
- Replace System.Data.SQLite.Core with Microsoft.Data.Sqlite 8.0.0 - Update DAS layer, NHibernate configuration, and database schema migrator - Update all project files to use Microsoft.Data.Sqlite package - Enables macOS ARM64 support and modern .NET compatibility
- Update SessionFactoryProvider to use SQLite20Driver (compatible with Microsoft.Data.Sqlite) - Add proper error handling and logging to SnapshotRunner for silent failures - Move CloseSession call to outer finally block to ensure cleanup in all scenarios - Add comprehensive test coverage for error scenarios - Resolves issue where convertSnapshot function failed silently on macOS
… native libraries Copy native SQLite libraries from runtimes folder to root before creating the R dependencies artifact for Microsoft.Data.Sqlite compatibility.
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the project from System.Data.SQLite.Core to Microsoft.Data.Sqlite, refactors simulation event handling to centralize it in InteractiveSimulationRunner, and adds support for resetting expression profile parameters. Key improvements include better error handling in the snapshot runner and updated build workflows.
- Replaced all SQLite package references from
System.Data.SQLite.CoretoMicrosoft.Data.Sqliteacross all project files - Centralized simulation event publishing (SimulationRunStartedEvent/FinishedEvent) in InteractiveSimulationRunner
- Added ResetExpressionParameterCommand for handling expression profile parameter resets
- Enhanced error handling and logging in SnapshotRunner
Reviewed Changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| Multiple .csproj files | Updated SQLite package from System.Data.SQLite.Core to Microsoft.Data.Sqlite v8.0.0 |
| DatabaseSchemaMigrator.cs | Updated SQLite class references from SQLiteConnection to SqliteConnection |
| DAS.cs | Updated provider factory initialization and data reading logic for Microsoft.Data.Sqlite |
| SessionFactoryProvider.cs | Updated SQLite connection string parameters |
| InteractiveSimulationRunner.cs | Centralized simulation event publishing with proper timing |
| IndividualSimulationEngine.cs | Removed event publishing (now handled by InteractiveSimulationRunner) |
| PopulationSimulationEngine.cs | Removed event publishing (now handled by InteractiveSimulationRunner) |
| StatusBarPresenter.cs | Updated to use InteractiveSimulationRunner for active simulation count |
| ParameterTask.cs | Added logic to use ResetExpressionParameterCommand for expression profile parameters |
| ResetParameterCommand.cs | Changed _oldValue visibility to protected for inheritance |
| ResetExpressionParameterCommand.cs | New command for resetting expression profile parameters |
| SnapshotRunner.cs | Enhanced error handling with better logging and exception handling |
| Build workflows | Updated code signing environment variables and R dependencies packaging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Resolved merge conflicts in: - SnapshotRunner.cs: Removed duplicate logging line - PKSim.Infrastructure.csproj: Kept Microsoft.Data.Sqlite, updated to V13 package versions - PKSim.csproj: Kept Microsoft.Data.Sqlite, updated to V13 versions, kept PostBuild target - PKSim.UI.Tests.csproj: Kept Microsoft.Data.Sqlite, updated to V13 versions - Deleted files removed in upstream/V13: - PKSimRDependencyResolution.csproj - PKSim.Matlab.Tests.csproj
…ration These properties (ContentSQLiteInteropFiles, CopySQLiteInteropFiles, etc.) were used by System.Data.SQLite.Core to handle native library deployment. Microsoft.Data.Sqlite handles native libraries through NuGet package assets automatically, so these properties are no longer needed.
- Resolved conflicts keeping Microsoft.Data.Sqlite migration - Removed Foreign Keys connection string parameter (Microsoft.Data.Sqlite has foreign keys enabled by default) - Kept manual data reading implementation in FillDataTable for Microsoft.Data.Sqlite compatibility - Preserved additional error handling test classes
This reverts commit 14b5b43.
- Add Content Include items to copy SQLite native libraries to output directory - Simplify workflow by removing PowerShell copy script, use -xr!runtimes flag
|
This won't build until we get a core merge, but this now works and only uses |
| var update = new SchemaUpdate(cfg); | ||
| update.Execute(useStdOut: false, doUpdate: true); |
There was a problem hiding this comment.
This won't work with Microsoft.Data.SQLite. Based on their public GitHub discussions, it won't work anytime soon.
We will have to manage migrations manually in the project database. That will also be required in MoBi.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/PKSim.Infrastructure/Serialization/DatabaseSchemaMigrator.cs
Outdated
Show resolved
Hide resolved
tests/PKSim.Tests/Infrastructure/SessionFactoryProviderSpecs.cs
Outdated
Show resolved
Hide resolved
msevestre
left a comment
There was a problem hiding this comment.
looks good to me with one question regarding weird command in one project file. Approving but please review just in case this is not intended


Closes #3325
Sister PRs:
Description
This PR migrates PK-Sim from System.Data.SQLite to Microsoft.Data.Sqlite. This change enables macOS ARM64 support, improves modern .NET compatibility.
Type of change
Please mark relevant options with an
xin the brackets.Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires documentation changes (link at least one user or developer documentation issue):
Algorithm update - updates algorithm documentation/questions/answers etc.
Other (please describe): Dependency migration.
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):