Skip to content

Fix deprecated=False override precedence for included routes#15183

Closed
dipenpadhiyar wants to merge 2 commits intofastapi:masterfrom
dipenpadhiyar:fix/router-deprecated-false-override
Closed

Fix deprecated=False override precedence for included routes#15183
dipenpadhiyar wants to merge 2 commits intofastapi:masterfrom
dipenpadhiyar:fix/router-deprecated-false-override

Conversation

@dipenpadhiyar
Copy link
Copy Markdown

@dipenpadhiyar dipenpadhiyar commented Mar 20, 2026

Summary

Fix deprecated precedence when including routers so an explicit deprecated=False
on a route is not overridden by a parent app or router with deprecated=True.

This aligns behavior with expected precedence: route-level configuration should
override parent settings.

Repro

from fastapi import FastAPI, APIRouter

child = APIRouter()

@child.get("/x", deprecated=False)
def x():
    return {"ok": True}

app = FastAPI(deprecated=True)
app.include_router(child)

assert "deprecated" not in app.openapi()["paths"]["/x"]["get"]

Changes

Preserve explicit non-None deprecated values when merging route, router, and app settings

Add a regression test for deprecated=False overriding parent deprecated=True

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 20, 2026 •
edited
Loading

Merging this PR will not alter performance

✅ 20 untouched benchmarks

Comparing dipenpadhiyar:fix/router-deprecated-false-override (2d746c9) with master (12bbd94)1

Open in CodSpeed

Footnotes

  1. No successful run was found on master (64feaec) during the generation of this report, so 12bbd94 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@dipenpadhiyar
Copy link
Copy Markdown
Author

dipenpadhiyar commented Mar 20, 2026

This PR fixes a logic bug in deprecated precedence for included routes.

Could a maintainer please add the bug label so the required label check can pass?

@YuriiMotov
Copy link
Copy Markdown
Member

YuriiMotov commented Mar 21, 2026

I'm not sure this is a bug.
Deprecating the whole router except some specific routes looks strange to me.

@dipenpadhiyar , what is your use case?

@nidhishgajjar
Copy link
Copy Markdown

nidhishgajjar commented Apr 14, 2026

Orb Code Review (powered by GLM 5.1 on Orb Cloud)

Review of PR #15183: Fix deprecated=False override precedence for included routes

Nice bug fix — correctly handles the case where a child route explicitly sets deprecated=False to override a parent's deprecated=True.

The bug ✅ correctly identified

Using or for precedence doesn't work here because False is falsy:

deprecated=False or self.deprecated=True  # → True (wrong!)

The fix ✅

Using is None checks properly distinguishes between "not specified" (None) and "explicitly set to False":

  • In add_api_route: deprecated=self.deprecated if deprecated is None else deprecated
  • In the router include: route-level takes highest precedence, then the include-level, then the router default

Test ✅

Good test case — verifies that a route with deprecated=False on a child router included in a FastAPI(deprecated=True) app does NOT appear as deprecated in the OpenAPI schema.

Minor readability note 💡

The nested ternary in the include logic works but is slightly hard to parse at a glance:

deprecated=(
    route.deprecated
    if route.deprecated is not None
    else self.deprecated
    if deprecated is None
    else deprecated
)

Adding parentheses could help future readers:

deprecated=(
    route.deprecated if route.deprecated is not None
    else (self.deprecated if deprecated is None else deprecated)
)

Not blocking — just a readability suggestion.

Summary

Clean, focused bug fix with test coverage. The is None check is the right approach to distinguish "unset" from "explicitly False."

Assessment: approve

@github-actions github-actions bot removed the waiting label Apr 14, 2026
@YuriiMotov YuriiMotov closed this Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dipenpadhiyar @YuriiMotov @nidhishgajjar