Repository navigation
Feature/scmu storage type picker - #6041
Draft
abparticular wants to merge 14 commits into
Draft
abparticular wants to merge 14 commits into
abparticular wants to merge 14 commits into
Conversation
Lets users pick RavenDB, SQL Server, or PostgreSQL as the storage backend when creating a new Error or Audit instance, mirroring the existing transport picker. The persistence manifest/factory seam already supported this at the engine level; the gap was entirely in the SCMU UI (hardcoded to RavenDB) and in the primary instance's config writer, which couldn't write arbitrary persister settings the way the audit config writer already could. The picker is gated behind the ServiceControl_IncludePersistencePicker environment variable (same pattern as ServiceControl_IncludeLearningTransport), since the SQL Server/PostgreSQL persisters are still marked not-production-ready. With the flag unset, RavenDB is auto-selected and the picker is hidden, so behavior is unchanged by default. Error and Audit instances get independent selections rather than a single shared picker. Scoped to net-new instance creation only - no upgrade/migration support, since no RavenDB-to-EF data migration tool exists yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The combo box was pre-selecting the alphabetically-first storage type (PostgreSQL) whenever the picker is shown, instead of leaving it unselected like the transport picker does. Only auto-select when there's no real choice to make (picker hidden, RavenDB-only); otherwise leave it null so the control shows its built-in "Please Select..." placeholder. Also adds a launch profile that sets ServiceControl_IncludePersistencePicker=true for local F5 testing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GetPersistencePathsToCleanUp() on the primary (Error) instance was a hardcoded list of RavenDB app.config keys, unlike the Audit instance's already-correct implementation which reads PersistenceManifest.Settings WithPathsToCleanup. This meant checking "remove database" on a SQL Server/PostgreSQL-backed Error instance silently deleted nothing, since none of the hardcoded RavenDB keys exist in its config. Made the Error instance manifest-driven to match Audit, and hoisted PersistenceManifest onto the shared ServiceControlBaseService base class so both the engine and the UI can ask a single HasCleanableStoragePaths question regardless of instance type. Also reworded the delete-instance confirmation dialog (and the PowerShell -RemoveDB help text) away from RavenDB-specific "DB subdirectory" language: when the storage type has nothing local to clean up, the checkbox is replaced with a note that the external database is not managed by ServiceControl and must be removed manually. Dropping an external SQL Server/PostgreSQL database automatically is out of scope here - it could be shared by other instances, and there is no existing mechanism in the persistence layer for it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…osen ShowErrorStorageConnectionString/ShowAuditStorageConnectionString (and the database-path-visibility equivalents) delegated directly to a computed property on the ServiceControl/ServiceControlAudit sub-view-model. Fody's PropertyChanged weaving only infers dependencies within the same class, and the sub-view-model's own PropertyChanged events get relayed to the parent as "Error<PropertyName>"/"Audit<PropertyName>", which doesn't match the "Show...Error.../Show...Audit..." naming used here - so WPF never found out the value had changed, and the connection string box (and the database path fields) never showed/hid themselves after a storage type change. Fixed by computing these directly from the already-correctly-relayed ErrorSelectedStorageType/AuditSelectedStorageType properties instead, matching the existing ShowErrorForwardingQueue pattern elsewhere in this file. Added a test that asserts PropertyChanged actually fires for the properties the XAML's Visibility bindings listen to, not just that the computed values are correct - the previous tests only read values directly and couldn't catch this class of bug. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Maintenance mode exists to start the embedded RavenDB server without the rest of the instance so RavenDB Studio can be used. An external RavenDB server, SQL Server, or PostgreSQL database is already reachable, so the user can stop the instance to do maintenance. The section stays visible while an instance is in maintenance mode so it can still be stopped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An instance with RavenDB/ConnectionString set keeps its data on a server ServiceControl doesn't manage. The remove dialog now shows the same "not managed by ServiceControl" message as SQL Server and PostgreSQL instead of the delete checkbox. RemoveDataBaseFolder also skips cleanup for unmanaged storage, so SCMU, PowerShell -RemoveDB, and unattended removal no longer delete the leftover local DBPath folder. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ServiceControlNewInstance gains a MessageBodyStorage setting that selects FileSystem, AzureBlob, or S3 storage and carries the values needed to reach it. The installer writes only the selected type's settings and validates the same combinations ServiceControl checks at startup, so a bad combination fails at install time instead of when the service starts. The message body folder is no longer derived from DBPath. SQL Server and PostgreSQL instances no longer create an unused DBPath folder, and the body folder is created with the service account's access, checked for existing content, and removed on uninstall when it is used. The remove dialog now offers to delete message body files for these instances and names anything ServiceControl does not manage: the database and, for Azure Blob Storage or Amazon S3, the message bodies. Until the Add Instance wizard offers a choice, it keeps file system storage in the previous location. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For SQL Server and PostgreSQL Error instances, the wizard now offers a message body storage choice of file system, Azure Blob Storage, or Amazon S3, and shows the settings needed to reach the selected storage: - File system: the message body path, defaulting to a MessageBodies folder next to the instance's DB and Logs folders. - Azure Blob Storage: a connection string, or a blob service URI with an optional managed identity client ID and authority host, plus the container name. - Amazon S3: bucket name, key prefix, region, service URL for S3-compatible storage, and an optional access key pair. Size and limit settings such as compression size and the free disk space threshold remain config-file only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The maintenance port is the embedded RavenDB server's port. The installer validated it for every new instance, so adding a SQL Server or PostgreSQL instance could fail when the default port was in use, and the wizard asked for a value those instances never use. The installer now validates the port only when the storage type declares it, and the wizard hides the field and its rules otherwise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
New-ServiceControlInstance and New-ServiceControlAuditInstance gain -PersistenceType, defaulting to RavenDB, and -DatabaseConnectionString. -DBPath and -DatabaseMaintenancePort are now required only for RavenDB. The choices are gated by ServiceControl_IncludePersistencePicker, like the SCMU storage type picker. New-ServiceControlInstance also gains -MessageBodyStorageType and the parameters needed to reach each storage type: -MessageBodyPath for the file system, -AzureBlob* for Azure Blob Storage, and -S3* for Amazon S3. Each cmdlet rejects parameters the selected persistence or message body storage type does not use, matching the existing transport connection string check. The help for New-ServiceControlInstance documents the new parameters. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ServiceControl_IncludePersistencePicker environment variable hid the SQL Server and PostgreSQL storage types in SCMU and PowerShell. They are now always offered alongside RavenDB, since this work ships with the next major version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Instance details showed a DB path for every Error and Audit instance, even when nothing uses it: SQL Server and PostgreSQL instances have no local database folder, and an external RavenDB server keeps its data on the server. The DB path now appears only for embedded RavenDB. An Error instance that stores message bodies on the file system shows its message body path in the same place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nstances The wizard passed the hidden database path for every Audit instance. A SQL Server or PostgreSQL Audit instance then got an unused DB folder, a "directory not empty" prompt for a field the user cannot see, and a folder that removal never cleans up. The Audit instance now gets a DB path only when its storage type uses one, as the Error instance does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both edit screens showed and required the database maintenance port, so saving a SQL Server or PostgreSQL instance failed validation: those instances have no port, which left the field empty. The Error edit view already bound the field's visibility to DatabaseMaintenancePortNumberRequired, but no such property existed, so the field always showed. Both edit view models now define that property from the instance's storage type. The Audit edit view binds to it too, both validators skip the port rules when it is false, and saving no longer converts an empty port to a number. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.