[DO NOT REVIEW] Add subscriptions and SqlonFhir View Definitions#5478
Open
[DO NOT REVIEW] Add subscriptions and SqlonFhir View Definitions#5478
Conversation
…icrosoft/fhir-server into feature/subscription-sqlonfhir
Wire MaterializationTarget through the full registration, adoption, sync, and materialization pipeline: - Add MaterializationTargetExtensionUrl constant for persisting target on Library resources (survives restarts, visible to other nodes) - Update IViewDefinitionSubscriptionManager.RegisterAsync/AdoptAsync to accept optional MaterializationTarget parameter (falls back to config DefaultTarget) - Inject SqlOnFhirMaterializationConfiguration into ViewDefinitionSubscriptionManager for resolving default target - Extract materialization-target extension from Library resources in both ViewDefinitionSyncService and ViewDefinitionLibraryRegistrationBehavior - Update ViewDefinitionRefreshChannel to use MaterializerFactory with per-registration target instead of hardcoded IViewDefinitionMaterializer - Update ViewDefinitionPopulationProcessingJob to use MaterializerFactory with per-registration target for bulk population - Persist materialization-target extension on Library resources alongside materialization-status - Update all affected unit tests for new constructor signatures Customers can now specify SqlServer, Parquet, or Fabric (Delta Lake) as the materialization target per-ViewDefinition via a FHIR extension on the Library resource. Without the extension, the server-wide DefaultTarget from SqlOnFhirMaterialization config is used. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add global target dropdown (SQL Server / Parquet / Fabric) in the ViewDefinition Registration card header - Add per-ViewDefinition target override dropdown (visible while Pending) - Show target badge (SQL/Parquet/Fabric) on each registered ViewDefinition - Update FhirDemoService.RegisterViewDefinitionAsync to accept optional target parameter and include materialization-target extension on the Library resource when specified - Add MaterializationTargetExtensionUrl constant to FhirDemoService Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
- Use @Bind:event='oninput' on the global target dropdown so the value updates instantly on selection (not on blur), preventing race conditions when clicking Register immediately after changing the dropdown - Include target comparison in ViewDefinitionSubscriptionManager's re-registration skip logic so changing only the target (with same ViewDefinition JSON) triggers a fresh registration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
When a ViewDefinition is registered with MaterializationTarget.Fabric but storage is not configured, the system now: - Fails fast at registration time with a clear error message instead of silently falling back to SQL Server materialization - Sets the Library resource status to 'error' with a descriptive message explaining what configuration is missing - Surfaces the error through the ViewDefinition status API Additional fixes: - SQL table creation is now conditional on target including SqlServer (Fabric/Parquet targets create their own storage structures) - Population orchestrator job carries the materialization target and skips SQL table creation for non-SQL targets - ViewDefinition status/list endpoints now include the Target property and only check SQL table existence for SqlServer targets - AdoptAsync no longer warns about missing SQL tables for non-SQL targets - MaterializerFactory.GetMaterializers() returns empty list instead of falling back to SQL; UpsertResourceAsync/DeleteResourceAsync throw InvalidOperationException when no materializers are available Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…zers Root cause: FhirJsonParser preserves meta.versionId from the fetched Library resource. When the modified Library is upserted back without a WeakETag, the stale version ID causes a silent version conflict in the data store, preventing the 'active' status from being persisted. Fix: Clear meta.versionId and meta.lastUpdated before upserting the Library resource, allowing the server to assign a new version. Upgraded error logging from Warning to Error level for better observability. Added test: GivenPopulationComplete_WhenHandled_ThenLibraryResourceWrittenWithActiveStatus verifies the Library upsert succeeds with cleared meta.versionId and the materialization-status extension is set to 'active'. Also added storage lifecycle methods (EnsureStorageAsync, StorageExistsAsync, CleanupStorageAsync) to IViewDefinitionMaterializer and all implementations, plus corresponding methods on MaterializerFactory, preparing to move SQL table management out of ViewDefinitionSubscriptionManager. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
ResourceProfileValidator.Validate threw NullReferenceException when fhirContext was null (background job context has no HTTP request). Added null-safe check on fhirContext?.RequestHeaders before accessing the profile validation header. ViewDefinitionRunHandler.RunFromMaterializedTableAsync now checks the registration target before querying SQL. For non-SQL targets (Fabric, Parquet), it returns a clear error explaining that \-run only supports SQL Server tables and to query the target storage directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
The processing job was falling back to DefaultTarget (SqlServer) when the in-memory registration was unavailable (e.g., after server restart). This caused Fabric-targeted ViewDefinitions to materialize into SQL tables. Fix: Add MaterializationTarget to ViewDefinitionPopulationProcessingJobDefinition and propagate it from the orchestrator job. The processing job now uses the target from the job definition as the primary source of truth, with in-memory registration as a secondary check for backward compatibility. Follow-up jobs also propagate the target. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…unresolvable ViewDefinitionRefreshChannel now fails with a clear error log and returns early if the registration is missing, instead of silently falling back to DefaultTarget (SqlServer). This prevents Fabric-targeted ViewDefinitions from accidentally materializing to SQL when the registration is unavailable. ViewDefinitionPopulationProcessingJob now uses the target directly from the job definition without any fallback logic — the target is always explicitly propagated through the job chain. DefaultTarget is now ONLY used when the user doesn't specify a per-ViewDefinition target (the null case in RegisterAsync/AdoptAsync). All other code paths use the explicitly specified target or fail with a descriptive error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The @CancelRequested output parameter from dbo.PutJobHeartbeat can return DBNull when the job row is not found or the column is NULL. The direct (bool) cast throws InvalidCastException. Handle DBNull by defaulting to false (no cancellation requested). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fabric requires a _last_checkpoint file to recognize Delta tables with many transaction log entries. Without it, tables appear as 'Unidentified'. CheckpointAsync is implemented only on DeltaLakeViewDefinitionMaterializer (not on the IViewDefinitionMaterializer interface) since checkpointing is a Fabric/Delta Lake-specific lifecycle concern. The subscription manager calls it directly when target includes Fabric after population completes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
5 participants
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.
Description
Describe the changes in this PR.
Related issues
Addresses [issue #].
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)