Fix card server crash when card count reaches max_cards#3068
Closed
Pr030304 wants to merge 1 commit into
Closed
Conversation
Contributor
Greptile SummaryThis PR fixes a Changes:
Confidence Score: 5/5
Important Files Changed
Reviews (1): Last reviewed commit: "Fix card server crash when card count re..." | Re-trigger Greptile |
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.
PR Type
Summary
Fix the card server crash that occurs when a run has at least
max_cardscards. The generator now stops cleanly at the configured limit instead of raisingStopIterationfrom inside the generator, which Python converts into aRuntimeError.Issue
Fixes #2946
Reproduction
Runtime: local
Commands to run:
Where evidence shows up: test output in the local console
Before (error / log snippet)
This happens because cards_for_run manually raises StopIteration once the card count reaches max_cards.
After (evidence that fix works)
Root Cause
cards_for_run in metaflow/plugins/cards/card_server.py is implemented as a generator. When the number of yielded cards reaches the configured max_cards, it manually raises StopIteration.
In Python 3, raising StopIteration from inside a generator body is not treated as a normal way to end iteration. Per PEP 479, it is converted into RuntimeError: generator raised StopIteration.
That means a run with enough cards to hit the limit can cause the card server to fail while iterating over cards, instead of stopping iteration cleanly.
There is also an off-by-one issue in the original logic: the counter is incremented before the limit check, so the generator attempts to stop as soon as the count reaches max_cards, rather than yielding up to that limit and then stopping.
Why This Fix Is Correct
The fix replaces the manual StopIteration with a normal generator return, which is the correct way to terminate a generator early.
This restores the intended invariant:
cards_for_run should yield at most max_cards cards
stopping at the limit should not raise an exception
runs below the limit should continue to behave unchanged
The change is intentionally minimal:
it does not alter card ordering
it does not change how cards are fetched
it only changes how the generator terminates when the limit is reached
Failure Modes Considered
Off-by-one behavior at the limit
The updated logic ensures the generator can yield exactly max_cards cards before stopping, instead of failing as soon as the count reaches the limit.
Behavior below the limit
The fix preserves the existing behavior for runs with fewer than max_cards cards. The added regression test verifies that all cards are still returned in that case.
Tests
Added regression tests in test/unit/test_card_server.py for:
Locally ran:
Non-Goals
This PR does not change:
AI Tool Usage