Repository navigation
fix: preserve attachment cancellation through storage errors - #2820
Draft
Rana Singh (ranadeepsingh) wants to merge 1 commit into
Draft
Rana Singh (ranadeepsingh) wants to merge 1 commit into
Rana Singh (ranadeepsingh) wants to merge 1 commit into
Conversation
Summary: - Check task/thread cancellation before and after attachment IO. - Recognize wrapped and suppressed interruption, including cyclic exception graphs, without treating storage timeouts as cancellation or replacing fatal errors. - Cover both legacy chat-completions and responses paths with local filesystem and HTTP fixtures; no live service calls. 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#2807 and microsoft#2801. Rationale: Attachment storage can wrap or clear interruption signals, causing a canceled Spark task to continue or return an ordinary row error. This follow-up preserves cancellation through file reads and cleanup while keeping storage timeouts sanitized and row-local. Local validation: sbt "set cognitive / Test / fork := false" "cognitive/compile" "cognitive/Test/compile" "cognitive/testOnly com.microsoft.azure.synapse.ml.services.openai.OpenAIPromptMultimodalRequestSuite" "cognitive/scalastyle" "cognitive/Test/scalastyle" Passed: 69 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
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.
Attachment storage can wrap or clear interruption signals, causing a canceled Spark task to continue or return an ordinary row error. This follow-up preserves interruption through file reads and cleanup while keeping recognizable storage timeouts sanitized and row-local.
Related Issues/PRs
Follow-up to #2807 and #2801.
What changes are proposed in this pull request?
Intentional limitation: a plain
InterruptedIOExceptionwith no timeout-specific type in its cause chain is treated as interruption, even when the thread flag was cleared and Spark has not marked the task canceled. If a connector uses that same exception shape for a timeout, the error fails the task rather than becoming a row error; Spark may retry the task and repeat requests already sent by that task. Exception messages such as"timeout"or"deadline reached"are not used to guess the cause. The chosen policy preserves interruption semantics rather than risking swallowing a real interruption. Typed timeouts and wrappers retaining a recognizable timeout cause remain row-local unless there is an independent interruption or cancellation signal.How is this patch tested?
Local validation covers the pending source corrections based on master
6c2699defcfb24d5bff5ba55f2252ba2d8cbea24. Those corrections are not yet all in the published PR head.Passed: compile, test compilation, 89/89 tests (0 skipped), production scalastyle, and test scalastyle. Run with JDK 8, Spark 3.5.0, Scala 2.12.17, and a single test JVM.
Draft status: local regression gates passed. The six-round review process, publication and Azure/GitHub CI for the latest pending source, and remote-comment reconciliation are not yet complete. Published as a draft first with explicit approval so review evidence can use the real PR number; no readiness gate is waived.
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.