Repository navigation
Conversation
Pipeline steps can stop execution by returning an HTTP response, but the dictionary-only annotation rejected valid redirect results. Align pipeline, disconnect, and partial annotations with this behavior and preserve callback result types without casts.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2068 +/- ##
=======================================
Coverage 96.30% 96.31%
=======================================
Files 370 370
Lines 20753 20797 +44
Branches 1207 1208 +1
=======================================
+ Hits 19987 20031 +44
Misses 565 565
Partials 201 201
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28d5a3d7b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def pipeline( | ||
| self, pipeline, pipeline_index: int = 0, *args, **kwargs | ||
| self, pipeline: list[str], pipeline_index: int = 0, *args, **kwargs |
There was a problem hiding this comment.
Use a sequence-compatible type rather than list[str] here. The built-in DEFAULT_AUTH_PIPELINE is a tuple, configured pipelines are commonly tuples, and the implementation only requires slicing and iteration, so backend.pipeline(DEFAULT_AUTH_PIPELINE) remains valid at runtime but is now incorrectly rejected by type checkers. The cast in BaseStrategy.get_pipeline() merely hides the same mismatch for internal calls instead of converting the tuple.
Useful? React with 👍 / 👎.
| def partial_step( | ||
| save_to_session: bool, allow_external_resume: bool = False | ||
| ) -> Callable[ | ||
| [Callable[..., dict[str, Any] | HttpResponseProtocol | None]], |
There was a problem hiding this comment.
Broaden the accepted callback return type to include falsy non-dictionary results. Pipeline steps returning False, 0, or "" are explicitly supported as no-op results—the wrapper normalizes them to {} and run_pipeline() continues—but this new decorator annotation makes type checkers reject an otherwise valid annotated partial step such as def step(...) -> bool: return False.
Useful? React with 👍 / 👎.
1 participant
Pipeline steps can stop execution by returning an HTTP response, but the dictionary-only annotation rejected valid redirect results. Align pipeline, disconnect, and partial annotations with this behavior and preserve callback result types without casts.