Repository navigation
perf: improve Fabric LRO polling - #303
Open
Alon Yeshurun (ayeshurun) wants to merge 20 commits into
Open
Alon Yeshurun (ayeshurun) wants to merge 20 commits into
Alon Yeshurun (ayeshurun) wants to merge 20 commits into
Conversation
## Summary Poll Fabric long-running operations at elapsed-time targets of 2, 7, and 20 seconds, then use the latest Retry-After header. Add a persistent disable_heuristic_lro_polling setting, defaulting to false, with documentation and a changelog entry. ## Prompting Intent Reduce completion detection latency for Fabric LROs without changing Jobs API polling. Allow users to restore header-only polling through configuration. ## Linked Sources - User requirements supplied in this session; no linked issue provided. - docs/essentials/settings.md ## Rationale Use monotonic deadlines so request duration does not accumulate into the early polling schedule. Keep Azure polling and HTTP 429 retry delays header-driven. Preserve the existing missing/invalid-header fallback. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Describe the Fabric LRO polling strategy without listing polling interval values. ## Prompting Intent Engineer asked to update the Changie entry to describe the approach rather than the polling interval. ## Linked Sources - User request in this session. ## Rationale The release note communicates adaptive heuristic polling followed by Retry-After behavior, and explains the opt-out setting, without embedding implementation timing details. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Shorten the changelog entry while retaining the adaptive polling approach and header-only option. ## Prompting Intent Engineer asked for a shorter Changie entry. ## Linked Sources - User request in this session. ## Rationale Remove implementation detail while preserving the user-facing behavior change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Remove the long-running operation polling explanation from the settings documentation. ## Prompting Intent Engineer asked to remove that section from docs/essentials/settings.md. ## Linked Sources - User request in this session. ## Rationale Keep the settings page focused on configuration inventory and management. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Replace the LRO polling explanation with one sentence and retain the enabling command. ## Prompting Intent Describe how setting disable_heuristic_lro_polling to true uses the LRO Retry-After header to wait between polls, without listing timing values. ## Linked Sources - User request in this session. ## Rationale Keep the configuration example concise and focused on the setting behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Apply the requested wording to the heuristic LRO polling configuration example. ## Prompting Intent Describe Retry-After polling generically, then explicitly state that the setting applies only to Fabric APIs. ## Linked Sources - User-provided wording in this session. ## Rationale Preserve the requested wording while making the API scope explicit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Move heuristic interval selection into get_polling_interval with optional keyword-only poll count and elapsed time. Keep operation state and Fabric configuration checks in the caller. ## Prompting Intent Engineer requested combining the heuristic calculation with the shared polling interval helper while preserving Jobs, Azure, and throttling behavior. ## Linked Sources - User request and agreed refactoring approach in this session. ## Rationale Explicit heuristic context keeps the helper stateless and existing callers header-driven. Custom intervals retain precedence, monotonic elapsed time preserves polling deadlines, and exhausted heuristics use Retry-After with the existing fallback. Add helper tests for precedence, fractional timing, missed deadlines, and fallback behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Add an LRO-specific polling interval wrapper that selects early heuristic delays for Fabric operations and delegates interval extraction to the generic helper. Keep Jobs, Azure, and HTTP 429 callers on the existing generic Retry-After path. ## Prompting Intent Engineer requested moving Fabric audience/config eligibility and elapsed-time heuristic selection out of the API client into a dedicated polling utility method. ## Linked Sources - User request and agreed approach in this session. ## Rationale A dedicated wrapper keeps get_polling_interval generic and centralizes LRO-specific policy without coupling unrelated polling callers to configuration. It uses elapsed deadlines for the first three Fabric polls and delegates all other cases to the existing Retry-After/fallback behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Add success/failure suffixes to polling tests introduced by this change and split successful terminal statuses from failed or cancelled operations. ## Prompting Intent Engineer requested that tests added in the PR end with _success or _failure according to the scenario. ## Linked Sources - User request in this session. ## Rationale Outcome-specific names make success and failure coverage explicit without renaming pre-existing tests. Fallback scenarios use _success because polling recovers successfully. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Move test_fab_lro_polling.py from test_core to test_utils without changing test behavior. ## Prompting Intent Engineer requested placing the LRO polling tests in test_utils. ## Linked Sources - User request in this session. ## Rationale Group polling regression coverage with the polling utility tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Remove the disable_heuristic_lro_polling setting and always apply the existing early polling schedule to eligible Fabric LROs. ## Prompting Intent Engineer asked to remove the new configuration setting from the LRO polling implementation. ## Linked Sources - User request in this session. ## Rationale Keeping heuristic polling unconditional removes an unsupported configuration branch while retaining the existing Retry-After behavior for later polls and non-Fabric audiences. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Clarify that adaptive polling helps detect completed Fabric long-running operations sooner. ## Prompting Intent Engineer asked to improve the Changie entry for adaptive LRO polling. ## Linked Sources - User request in this session. ## Rationale Describe the user-visible improvement as earlier completion detection, not faster server-side execution, and retain the Retry-After behavior in the explanation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Simplify the Changie entry to describe the improvement to Fabric LRO polling intervals. ## Prompting Intent Engineer asked for changelog wording similar to 'improve fabric lro polling intervals'. ## Linked Sources - User request in this session. ## Rationale Use concise wording that communicates the optimization without over-specifying its implementation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Restore integer custom polling intervals and round heuristic waits in the LRO-specific helper. Add debug traces for poll number, elapsed time, selected delay, and scheduling source, with regression coverage. ## Prompting Intent Engineer requested polling-hop debug traces and simplifying get_polling_interval back to integer intervals, then asked to push the changes. ## Linked Sources - User requests in this session. - Draft PR: #108 ## Rationale Keep the generic interval helper integer-only and confine deadline rounding to LRO scheduling. Rounding permits up to half a second of target deviation while retaining latest-response Retry-After behavior. Debug traces expose scheduling decisions without adding operation URLs or payloads. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Restore the direct elapsed-deadline polling calculation with fractional waits and remove polling-hop debug traces. Update helper types and regression expectations accordingly. ## Prompting Intent Engineer supplied the exact polling implementation to restore and requested pushing the changes. ## Linked Sources - User-provided implementation in this session. - Draft PR: #108 ## Rationale Preserve fractional remaining delays without rounding, clamp missed deadlines to zero, and delegate later polls to the latest response Retry-After header. Existing generic interval debug logging remains unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Restore integer polling interval types and round remaining heuristic waits to whole seconds. Keep the direct polling helper flow without hop traces. ## Prompting Intent Engineer requested explicit elapsed and target variables with max(0, round(target - elapsed)), corresponding integer interval types, and pushing the changes. ## Linked Sources - User-provided calculation in this session. - Draft PR: #108 ## Rationale Keep the generic polling helper integer-only while confining elapsed-time calculations and rounding to the LRO helper. Rounding can shift early poll targets by up to half a second; later polls retain latest-response Retry-After behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Clarify that polling waits until the next target or proceeds immediately when that target has passed. ## Prompting Intent Engineer requested rephrasing the polling calculation comment and pushing the change. ## Linked Sources - User request in this session. - Draft PR: #108 ## Rationale Explain the purpose of the nonnegative wait directly without changing polling behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped with targeted regression coverage and no identified blocking issues; tests were not executed.
No open issues 🎉
What changed in this PR
This PR improves Fabric long-running operation polling to detect completion sooner while preserving other polling flows.
Changes:
- Targets the first three Fabric polls at approximately 2, 7, and 20 seconds, accounting for request duration.
- Uses the latest response’s
Retry-Aftervalue for subsequent polls. - Adds timing and regression tests, plus a release note.
| File | Description |
|---|---|
tests/test_utils/test_fab_lro_polling.py |
Tests polling timing, terminal results, and unchanged Azure and job intervals. |
tests/test_utils/test_fab_http_polling_utils.py |
Tests elapsed-time targets and header fallback behavior. |
src/fabric_cli/utils/fab_http_polling_utils.py |
Adds Fabric-specific early polling interval selection. |
src/fabric_cli/client/fab_api_client.py |
Integrates elapsed-time tracking and latest-response polling intervals. |
.changes/unreleased/optimization-20261018-095356.yaml |
Documents the polling optimization. |
Review effort: Balanced
## Summary Simplify the simulated polling clock, remove the artificial time offset, and clarify test names and expected outcomes without reducing coverage. ## Prompting Intent Engineer requested clearer polling tests and names, using heuristic rather than numeric targets in the first test name, then asked to push the changes. ## Linked Sources - User requests in this session. - Draft PR: #108 ## Rationale Named clock attributes and explicit expected values make timing and failure scenarios easier to follow. Descriptive test names retain success/failure suffixes and state the behavior under test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Remove the separate LRO polling test file and add explicit non-Fabric audience and latest Retry-After cases to the HTTP polling utility tests. ## Prompting Intent Engineer requested removing the separate LRO polling tests, retaining simple helper-level coverage in test_fab_http_polling_utils.py, and pushing the changes. ## Linked Sources - User requests in this session. - Draft PR: #108 ## Rationale Keep interval-selection tests in one file with explicit audience and header cases. This intentionally removes the separate poll-loop regression scenarios in favor of the requested narrower unit-test scope. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary Add caller-level polling regression tests covering Fabric heuristic waits, request duration, latest-response Retry-After values, and unchanged Azure header-based timing. ## Prompting Intent Engineer requested addressing the upstream review comment about missing polling orchestration coverage, then committing all workspace changes without pushing. ## Linked Sources - Review comment: microsoft#303 (comment) - User instructions in this session. ## Rationale Exercise the real polling loop with mocked requests, sleep, and monotonic time to catch poll-count and stale-header regressions that helper-only tests cannot detect. Keep the tests self-contained without production changes or live API calls. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
📥 Pull Request
✨ Description of new changes
Improve Fabric long-running operation polling so completion can be detected sooner:
Elapsed-time targets account for request duration - if a target has passed, the next poll starts immediately. Fabric's Job Scheduling, OneLake DFS and Azure polling are unchanged.
Examples: