Repository navigation
fix: honor assigned Spark sessions in nested pipeline persistence - #2818
Open
Rana Singh (ranadeepsingh) wants to merge 2 commits into
Open
Rana Singh (ranadeepsingh) wants to merge 2 commits into
Rana Singh (ranadeepsingh) wants to merge 2 commits into
Conversation
Summary: - Propagate assigned sessions through individual and array pipeline serializers. - Restore the previous active session on success and failure; preserve the default session and existing no-argument/null-session behavior. - Retain the merged legacy-overload tests and add nested persistence and policy regressions without changing serialized formats. Prompting intent: Preserve reviewed follow-up fixes after the prerequisite PRs were merged, and complete current-source validation and review. Linked sources: Follow-up to microsoft#2809 and microsoft#2803. Rationale: Nested pipeline parameters can fall back to another active Spark session instead of the session assigned to their reader or writer. That can select the wrong configuration or legacy-deserialization policy. This follow-up carries the assigned session through nested stages and restores the caller's active session. Local validation: sbt "set core / Test / fork := false" "core/compile" "core/Test/compile" "core/testOnly com.microsoft.azure.synapse.ml.core.serialize.ValidateComplexParamSerializer com.microsoft.azure.synapse.ml.stages.VerifyUDFTransformerPersistence" "core/scalastyle" "core/Test/scalastyle" Passed: 29 tests, 0 skipped, compile/Test compile, both scalastyle gates. Publication: Explicitly authorized draft-first bootstrap; formal reviews and current-head CI remain pending. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ba39157-c8dd-4adb-90e0-ff963a1e6733
|
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). |
3 of 5 tasks
## Summary Add direct coverage for the existing no-argument and null-session serializer fallback, including active/default session preservation, and retain the independent review evidence. No production behavior changes in this commit. ## Prompting Intent Complete validation, independent reviews, CI and comment reconciliation for the remaining SynapseML pull requests after the prerequisites were merged. Preserve legacy behavior and public signatures; do not merge any pull request. ## Linked Sources - microsoft#2818 - microsoft#2809 - microsoft#2803 - reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-3-grok-4.7-slice1.md - reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-6-grok-4.7-slice1.md ## Rationale Keep fallback behavior covered at the public serializer boundary rather than changing it in response to an incorrect baseline assumption. The baseline and current runtime agree in 18 absent-session cases; review rebuttals preserve the original findings and document this evidence. ## Validation sbt "set core / Test / fork := false" "core/compile" "core/Test/compile" "core/testOnly com.microsoft.azure.synapse.ml.core.serialize.ValidateComplexParamSerializer com.microsoft.azure.synapse.ml.stages.VerifyUDFTransformerPersistence" "core/scalastyle" "core/Test/scalastyle" Passed: 31 tests, 0 skipped, production/test compilation and both style gates. Six independent three-family review rounds completed with explicit dispositions. New-head CI, remote-comment reconciliation and the final CI-qualified review remain separate gates; the pull request remains a draft. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ba39157-c8dd-4adb-90e0-ff963a1e6733
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2818 +/- ##
==========================================
+ Coverage 88.03% 88.04% +0.01%
==========================================
Files 348 348
Lines 21696 21708 +12
Branches 2303 2315 +12
==========================================
+ Hits 19100 19113 +13
+ Misses 2596 2595 -1
🚀 New features to boost your workflow:
|
Rana Singh (ranadeepsingh)
marked this pull request as ready for review
November 9, 2026 20:47
Rana Singh (ranadeepsingh)
requested a review
from Brendan Walsh (BrendanWalsh)
as a code owner
November 9, 2026 20:47
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the nested pipeline session-propagation change.
No open issues 🎉
What changed in this PR
This PR fixes nested pipeline persistence in SynapseML so child stages use the Spark session assigned to their reader or writer, including its legacy-deserialization policy, without changing serialized formats.
Changes:
- Forward assigned sessions through single-stage and array serializers, restoring the caller’s active session after success or failure.
- Add shared fixtures and regression coverage for session selection, overwrite, fallback behavior, and deserialization-policy enforcement.
- Clarify legacy helper comments and retain review reports with resolution records.
The PR reports 31 passing local tests plus compilation and style checks. These were not rerun here; new-head CI remains pending in the description.
| File | Description |
|---|---|
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-6-grok-4.7-slice1.md |
Records hardening review and rejected-finding rationale. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-6-gpt-6.1-sol-slice1.md |
Records final hardening checks. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-6-claude-opus-5.5-slice1.md |
Records compatibility and hardening checks. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-5-grok-4.7-slice1.md |
Records updated coverage checks. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-5-gpt-6.1-sol-slice1.md |
Records persistence test review. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-5-claude-opus-5.5-slice1.md |
Records coverage and test-isolation analysis. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-4-grok-4.7-slice1.md |
Records session-routing correctness checks. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-4-gpt-6.1-sol-slice1.md |
Records persistence correctness review. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-4-claude-opus-5.5-slice1.md |
Records detailed session and compatibility analysis. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-3-grok-4.7-slice1.md |
Preserves fallback concern and rejection evidence. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-3-gpt-6.1-sol-slice1.md |
Records exception and boundary checks. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-3-claude-opus-5.5-slice1.md |
Records restoration and fallback analysis. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-2-grok-4.7-slice1.md |
Records architecture and convention checks. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-2-gpt-6.1-sol-slice1.md |
Records delegation and format compatibility checks. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-2-claude-opus-5.5-slice1.md |
Records API and fixture-design analysis. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-1-grok-4.7-slice1.md |
Records initial session-propagation review. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-1-gpt-6.1-sol-slice1.md |
Records initial correctness and coverage checks. |
reviews/pr-2818/task-unknown-attempt-11-stage-attempt-1-review-1-claude-opus-5.5-slice1.md |
Records initial security and compatibility analysis. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-2-review-4-gpt-6.1-sol-slice1.md |
Retains repeated correctness-review evidence. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-5-grok-4.7-slice1.md |
Tracks resolved overwrite and fallback coverage gaps. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-4-grok-4.7-slice1.md |
Retains earlier correctness checks. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-4-gpt-6.1-sol-slice1.md |
Retains earlier persistence review. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-4-claude-opus-5.5-slice1.md |
Retains detailed session-contract analysis. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-3-grok-4.7-slice1.md |
Retains earlier failure-path checks. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-3-gpt-6.1-sol-slice1.md |
Retains earlier restoration and boundary review. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-3-claude-opus-5.5-slice1.md |
Retains concurrency and fallback analysis. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-2-grok-4.7-slice1.md |
Retains architecture and convention review. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-2-gpt-6.1-sol-slice1.md |
Retains delegation and compatibility review. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-2-claude-opus-5.5-slice1.md |
Retains API and shared-fixture analysis. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-1-grok-4.7-slice1.md |
Retains initial routing and security checks. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-1-gpt-6.1-sol-slice1.md |
Retains initial persistence review. |
reviews/pr-2818/task-unknown-attempt-10-stage-attempt-1-review-1-claude-opus-5.5-slice1.md |
Retains initial compatibility and coverage analysis. |
core/src/test/scala/com/microsoft/azure/synapse/ml/stages/VerifyUDFTransformerPersistence.scala |
Tests nested reader policies and session restoration. |
core/src/test/scala/com/microsoft/azure/synapse/ml/core/serialize/ValidateComplexParamSerializer.scala |
Tests assigned sessions, overwrite, fallback, and failures. |
core/src/test/scala/com/microsoft/azure/synapse/ml/core/serialize/ComplexParamTestFixtures.scala |
Shares session-sensitive stages and cleanup helpers. |
core/src/main/scala/org/apache/spark/ml/Serializer.scala |
Propagates assigned sessions and scopes active-session changes. |
core/src/main/scala/org/apache/spark/ml/ComplexParamsSerializer.scala |
Adds comments clarifying legacy helper session selection. |
Review effort: Balanced
This branch has not been deployed
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.
Nested pipeline parameters can fall back to another active Spark session instead of the session assigned to their reader or writer. That can select the wrong configuration or legacy-deserialization policy. This follow-up carries the assigned session through nested stages and restores the caller's active session.
Related Issues/PRs
Follow-up to #2809 and #2803. This independent fix should land before #2811.
What changes are proposed in this pull request?
How is this patch tested?
Local validation covers the source published at
b21f52d982909ad1560d3143fadd4cb7057dbdc2, based on master6c2699defcfb24d5bff5ba55f2252ba2d8cbea24:Passed: compile, test compilation, 31/31 tests (0 skipped), production scalastyle, and test scalastyle. Run with JDK 11, Spark 3.5.0, Scala 2.12.17, and a single test JVM. The tested source patch was checked byte-for-byte before committing; the commit also records review reports.
The six-round, three-reviewer pre-commit review is complete. Original reports and explicit resolutions are retained under
reviews/pr-2818/. Repeated fallback findings were rejected using an 18-case baseline/current runtime comparison: both revisions already create and retain sessions when none exists. The original reviewer verdicts were not rewritten as clean.Current-head CI: Azure validation completed successfully for the head and master base above. All 31 required regression cases passed. The causal job passed its one authorized infrastructure retry; previously successful jobs and test publications were retained. GitHub build, style, dependency, and website checks passed.
The CI warnings were inspected rather than treated as clean: 24 hosted disk warnings, three non-gating setup-telemetry warnings (the affected OpenCV Python job published four passing tests), and one empty website-coverage warning permitted by the effective pipeline. Component Governance reported the same 12 dependency alerts as the target build, including three high-severity alerts. Those alerts remain unresolved; incomplete component detection and pending background analysis mean this is not a security clearance.
InternalCompatandFabricE2Ewere intentionally skipped by the fork-PR conditions, not counted as passes.Review status: marked ready for review after the CI evidence was qualified. Copilot reviewed this exact head and reported no active issues; all remote review threads were checked and none are open. The final CI-qualified independent review and its artifact publication remain pending, so this is not yet a merge-ready claim. No merge or auto-merge has been performed.
Does this PR change any dependencies?
Does this PR add a new feature? If so, have you added samples on website?
website/docs/documentationfolder.Make sure you choose the correct class
estimators/transformersand namespace.DocTablepoints to correct API link.npm startto make sure the website renders correctly.<!--pytest-codeblocks:cont-->before each python code blocks to enable auto-tests for python samples.WebsiteSamplesTestsjob pass in the pipeline.