fix: safely clean up Fabric test artifacts after 24 hours - #2728
Open
Rana Singh (ranadeepsingh) wants to merge 4 commits into
Open
Rana Singh (ranadeepsingh) wants to merge 4 commits into
Rana Singh (ranadeepsingh) wants to merge 4 commits into
Conversation
Rana Singh (ranadeepsingh)
requested a review
from Brendan Walsh (BrendanWalsh)
as a code owner
September 18, 2026 23:30
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey Rana Singh (@ranadeepsingh) 👋! We use semantic commit messages to streamline the release process. Examples of commit messages with semantic prefixes:
To test your commit locally, please follow our guild on building from source. |
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Copilot started reviewing on behalf of
Rana Singh (ranadeepsingh)
September 18, 2026 23:31
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Destructive infrastructure cleanup paths require final human review, and validation records contain unresolved inconsistencies.
Review effort: Lite
Findings: None
What changed in this PR
Updates Fabric test artifact cleanup to safely remove only owned, stale artifacts with dependency checks, dry-run support, and deletion confirmation.
Changes:
- Adds conservative 24-hour ownership and activity filtering.
- Adds guarded deletion, pagination, failure aggregation, and regression tests.
- Documents cleanup operation and records review validation.
| File | Summary |
|---|---|
reviews/task-fabric-cleanup-attempt-1-review-6-claude-opus-5.md |
Final review and validation record. |
reviews/task-fabric-cleanup-attempt-1-review-5-gemini-3.8-flash.md |
Testing review and resolutions. |
reviews/task-fabric-cleanup-attempt-1-review-4-gpt-6-astra.md |
Failure-aggregation review record. |
reviews/task-fabric-cleanup-attempt-1-review-3-claude-opus-5.md |
Edge-case findings and resolutions. |
reviews/task-fabric-cleanup-attempt-1-review-2-gemini-3.8-flash.md |
Architecture review record. |
reviews/task-fabric-cleanup-attempt-1-review-1-gpt-6-astra.md |
Initial review findings. |
docs/Reference/Developer Setup.md |
Documents cleanup usage and safeguards. |
core/src/test/scala/com/microsoft/azure/synapse/ml/nbtest/FabricTestArtifactTrackerSuite.scala |
Adds cleanup safety regressions. |
core/src/test/scala/com/microsoft/azure/synapse/ml/nbtest/FabricNotebookTests.scala |
Integrates cleanup and dry-run handling. |
core/src/test/scala/com/microsoft/azure/synapse/ml/nbtest/FabricArtifactCleanup.scala |
Implements guarded cleanup logic. |
core/src/test/scala/com/microsoft/azure/synapse/ml/fabric/FabricOperations.scala |
Integrates Fabric APIs and ownership metadata. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
## Summary Replace name-only three-day cleanup with ownership-aware, UTC-based 24-hour retention for Fabric E2E artifacts. Confirm asynchronous deletion before reclaiming stores, and add deterministic safety regressions and operator docs. ## Prompting Intent Investigate failing Fabric E2E pipelines, delete workspace items created by this repository's tests only when older than 24 hours, and publish the change as a separate PR. Preserve unrelated, active, shared, and ambiguous items. ## Linked Sources - Existing cleanup and suite selection: https://github.com/microsoft/SynapseML/blob/cd45147c7025f483e86fc028069d72b070e73a55/core/src/test/scala/com/microsoft/azure/synapse/ml/nbtest/FabricNotebookTests.scala - Existing Fabric adapter and artifact creation: https://github.com/microsoft/SynapseML/blob/cd45147c7025f483e86fc028069d72b070e73a55/core/src/test/scala/com/microsoft/azure/synapse/ml/fabric/FabricOperations.scala - Operator documentation: docs/Reference/Developer Setup.md - Six sequential review artifacts: reviews/task-fabric-cleanup-attempt-1-review-*.md - Job history contract: https://learn.microsoft.com/en-us/rest/api/fabric/core/job-scheduler/list-item-job-instances - Schedule contract: https://learn.microsoft.com/en-us/rest/api/fabric/core/job-scheduler/list-item-schedules - Spark job type: https://learn.microsoft.com/en-us/rest/api/fabric/sparkjobdefinition/background-jobs/run-on-demand-spark-job-definition ## Rationale Artifact names alone do not establish safe ownership or activity. Require matching ownership metadata, expired creation/update timestamps, safe dependencies, and idle unscheduled jobs. Re-read inventory before writes; poll deletion visibility rather than repeat DELETE. Retain stores on any unconfirmed job deletion, and report independent failures explicitly. Keep the change in test infrastructure without altering public APIs or CI configuration. This infrastructure bug fix carries the area/build PR label. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
Rana Singh (ranadeepsingh)
force-pushed
the
fix/fabric-test-cleanup-24h-20260918
branch
from
September 19, 2026 00:14
a016ddb to
1205df2
Compare
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Copilot started reviewing on behalf of
Rana Singh (ranadeepsingh)
September 19, 2026 00:15
View session
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
AB#5628913 ## Summary Move the six existing Fabric cleanup review artifacts into reviews/pr-2728/ without changing their contents or the reviewed implementation. ## Prompting Intent The engineer requested reviews/pr-<pr_number>/ for review artifacts and an assigned ADO work item in the current sprint linked to this pull request. ## Linked Sources - Pull request: microsoft#2728 - Work item: https://dev.azure.com/msdata/A365/_workitems/edit/5628913 - Existing review rounds: reviews/pr-2728/task-fabric-cleanup-attempt-1-review-*.md ## Rationale Scope the audit trail to its PR while preserving all original review findings, resolutions, and evidence. Git blob checks confirm six content-identical moves. No source code, dependencies, or CI configuration changes are needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Copilot started reviewing on behalf of
Rana Singh (ranadeepsingh)
September 19, 2026 00:25
View session
Comment on lines
+49
to
+50
| case JsObject(fields) => fields.values.flatMap(references).toSet | ||
| case JsArray(values) => values.flatMap(references).toSet |
AB#5628913 ## Summary Separate the existing Fabric cleanup from E2E execution into a named, fail-closed pipeline task after authentication and build setup. Preserve cleanup reports across the second SBT invocation and publish staged evidence on failure. Defer Fabric workspace lookup, store allocation and notebook submission until test execution. Cache per-suite cleanup and setup outcomes, preserve interrupts, and retain bounded parallel notebook execution and tracked artifact cleanup. ## Prompting Intent The engineer asked to make repository-owned test-item cleanup a pre-job task before Fabric E2E rather than hiding it inside the notebook task. They approved the explicit pipeline preflight plus lazy suite-level guards. Retain the strict 24-hour ownership policy in the separate cleanup PR, link their current-sprint ADO task, and keep all review artifacts under reviews/pr-2728/. ## Linked Sources - Pull request: microsoft#2728 - Tracking task: https://dev.azure.com/msdata/A365/_workitems/edit/5628913 - Operator guide: docs/Reference/Developer Setup.md - Review findings and resolutions: reviews/pr-2728/task-5628913-attempt-2-review-*.md ## Rationale A separate step exposes cleanup failures without introducing another job or duplicating setup. Per-suite checks protect direct runs independently of CI. Cached setup failures prevent later tests from retrying allocations. Laziness allows test registration without connecting to Fabric while preserving the existing parallel workload and public suite member types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Copilot started reviewing on behalf of
Rana Singh (ranadeepsingh)
September 19, 2026 02:19
View session
AB#5628913 ## Summary Make NotebookFixture abstract and create anonymous concrete instances where its owning suite executes it. Preserve the repository-wide CI test coverage gate. ## Prompting Intent The engineer reported failed Azure jobs on the Fabric cleanup PR and supplied the test-matrix and TLS errors. Investigate each failed job, repair the introduced fixture regression, and distinguish unrelated infrastructure and port prerequisites. ## Linked Sources - PR: microsoft#2728 - Failed build: https://dev.azure.com/msdata/A365/_build/results?buildId=236598594 - Task: https://dev.azure.com/msdata/A365/_workitems/edit/5628913 - Port prerequisite: microsoft#2725 - Review: reviews/pr-2728/task-5628913-attempt-3-review-1-gpt-6-astra.md ## Rationale The private helper is exercised by its parent suite, not a standalone test entry point. Making that intent explicit fixes source discovery without adding a fake CI selector or weakening the guard. Preserve TLS validation and strict port replay; those failures have separate infrastructure and branch-content causes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
CI failure triage for build 236598594:
Evidence and the direct review are in reviews/pr-2728/task-5628913-attempt-3-review-1-gpt-6-astra.md. Fresh validation will check the core fix and retry infrastructure setup, but the release prerequisite remains an explicit blocker. |
Copilot started reviewing on behalf of
Rana Singh (ranadeepsingh)
September 19, 2026 05:53
View session
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
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
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.
Related Issues/PRs
Separate infrastructure fix for Fabric E2E failures caused by workspace artifact
quota exhaustion.
ADO task: AB#5628913.
What changes are proposed in this pull request?
Replace the existing name-only, three-day cleanup with a strict 24-hour policy
for artifacts owned by this repository's Fabric tests.
UTC. Tag new test artifacts with an explicit OSS ownership description and
conservatively recognize legacy names and descriptions.
items. Re-read inventory before deleting; keep stores whenever job ownership
or consumer relationships do not establish safe deletion.
two-second pauses instead of assuming a successful DELETE is immediately
visible. A concurrent not-found response still requires confirmed absence.
and report collected failures. Let Fabric cascade managed SQL endpoints
rather than deleting endpoints independently.
SYNAPSEML_FABRIC_CLEANUP_DRY_RUN=true, operator documentation, and safetyregressions in an existing CI-selected suite.
Fabric cleanup preflighttask inside the existingFabricE2E job, after authentication/build setup and before the success-gated
E2E task. Preserve cleanup JUnit results before the next SBT invocation and
publish staged reports and phase metadata even when E2E is skipped or fails.
execution. Each suite independently caches its cleanup and setup outcomes,
including interrupted cleanup. Failed setup cannot retry allocations for
later tests. Successful runs retain concurrency 3, immediate per-job cleanup,
and executor shutdown before final artifact cleanup.
This changes OSS test infrastructure and the Fabric pipeline task layout.
Public main APIs, generated code, workflows, dependency pins, and other
repositories are unchanged.
How is this patch tested?
before implementing the guards and pipeline split.
FabricTestArtifactTrackerSuiteandFabricArtifactNamesSuite, with no skipped tests.compileandTest/compile.scalastyleandTest/scalastyle.with fake commands for success, cleanup failure, E2E failure, and failed setup.
follow-up. Findings and resolutions are committed under
reviews/pr-2728/.content-preserving rebase and preflight orchestration follow-up.
Both live passes examined 24 items, found 0 owned jobs and 0 owned
stores, and confirmed 0 deletions. No unrelated items were touched, and
these passes did not reclaim capacity.
The live passes did not exercise the real deletion/confirmation, job-history,
or schedule paths because no items were eligible. Deterministic tests cover
those safeguards. Endpoint shapes and
sparkjobwere checked against the officialhistory,
schedule,
and Spark job type
contracts.
The new preflight orchestration has not yet completed a live Fabric CI run.
Local checks and a queued Azure build do not establish that the original
Fabric E2E failures are resolved.
Does this PR change any dependencies?
Does this PR add a new feature? If so, have you added samples on website?
docs/Reference/Developer Setup.md.