Repository navigation
Deprecate Application(env=...) for removal in 1.0 - #318
Open
claude-tmgbedu wants to merge 2 commits into
Open
claude-tmgbedu wants to merge 2 commits into
claude-tmgbedu wants to merge 2 commits into
Conversation
The deployment environment belongs outside the code: APP_ENV, the CLI --env option, or pytest detection. Passing env= now emits a DeprecationWarning at the caller; behaviour is unchanged until 1.0. Framework tests stop passing the redundant env="testing" and pytest treats this warning as an error so it cannot creep back in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgHs6PRFJehnb1BbCNtLww
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
claude-tmgbedu
commented
Oct 25, 2026
claude-tmgbedu
left a comment
Author
There was a problem hiding this comment.
Review + QA: PR #318 (deprecate Application(env=...))
Verdict: approve. No blocking issues, two non-blocking findings.
QA (own worktree at 6da18db)
cd fastapi_startkit && uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3300 passed, 10 skipped, 187 subtests, coverage 91.50% (threshold 80).example/config-apptests: 12 passed, also with-W error::DeprecationWarning. They need the gitignored.env; copy.env.exampleto.envfirst. Without it, 11 fail with "Unable to determine environment", which is not caused by this PR.- Scratch app with
-W default::DeprecationWarning:Application(base_path=..., env="production")in the script warns at the script line.- The same call in a factory module (
create_app) warns at the factory's line. class MyApp(Application): passcalled withenv=warns at the caller.
artisan --env=staging listand--env staging list: env isstaging,--envtokens are removed fromsys.argv, no warning.APP_ENV=productionalone: no warning.- Pytest: no warning without
env=. The new tests coverAPP_ENV,--env xand--env=x.
Review
- Warning fires once per construction, only when
env is not None. Nothing insrc/, the installer stubs orexample/*passesenv=.set_environment()is untouched. Environment.resolve_environmentis untouched, so precedence is unchanged: pytest, thenenv=, thenAPP_ENV(which--envsets), then.env. Note thatenv=also outranks--env, not onlyAPP_ENV. ADR 040 mentions onlyAPP_ENV.- Leaving
set_environment()alone is right. It has no callers insrc/, mutates a live app instead of putting a deployment environment in the bootstrap file, and ADR 040 says why. - The
filterwarningsentry is in the framework's ownpyproject.tomlonly. It is scoped to this one message, so it neither affects downstream suites nor hides other warnings. Downstream suites that passenv="testing"get a normal warning. - ADR 040 exists, the number is unused on main, and it is indexed in
docs/adr/AGENTS.md. Other open PRs use 035-039, so there is no collision. No comments or docstrings were added to source or tests.
Findings (non-blocking)
- The warning is invisible with Python's default filters for the usual layout.
fastapi_startkit/src/fastapi_startkit/application.py:46-52.DeprecationWarningis shown by default only when attributed to__main__. Repro:bootstrap/factory.pywithApplication(..., env="production"), imported from a script, run withpython main.py. It prints nothing; only-W default::DeprecationWarningor pytest shows it. Apps built onbootstrap/application.py, including underuvicorn --factory, will not see it. ConsiderFutureWarningor a documented filter, if the PM wants the warning seen.DeprecationWarningitself is acceptable and conventional. - Subclasses that override
__init__get the wrong location.application.py:51. Withclass MyApp2(Application): def __init__(self, *a, **kw): super().__init__(*a, **kw), the warning points at thesuper().__init__line, not the user'sMyApp2(env=...)call. This is a limit of fixedstacklevel. Subclasses without an__init__override are fine.
Nits:
example/config-app/tests/test_config_app.pyrewords the existing comments instead of dropping them. This is harmless but conflicts with the no-comments rule.pyproject.tomlstays at 0.67.0. A release bump is presumably handled separately.
Keep ADR 038 and 040 rows in docs/adr/AGENTS.md and drop env= from the new Vite request scope test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Summary
Application(env=...)now emits aDeprecationWarning(stacklevel=2, so it points at the caller). The warning says the argument will be removed in 1.0 and to useAPP_ENVor--envinstead. Resolution order and behaviour are unchanged.APP_ENVvariable, the CLI--env <name>/--env=<name>option, or pytest auto-detection (testing).src/, theapplication/starter, the installer stubs or the example bootstraps passedenv=, so the framework itself never triggers the warning.env="testing". Pytest'sfilterwarningsturns this deprecation into an error, so the suite can't bring it back.example/config-apptests now select environments throughAPP_ENV.set_environment()is unchanged. It switches the environment at runtime and does not hard-code one in the bootstrap file.Tests
tests/core/test_application_env_deprecation.pychecks that:env=warns, and the warning points at the caller's file;env=still selects the environment;APP_ENV,--env xand--env=xwork without a warning and remove the--envtokens from argv.uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3300 passed, 10 skipped, coverage 91.50%.example/config-app: 12 passed.Docs follow-up: a separate docs-repo PR will update
docs/configuration.md. It depends on this PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01QgHs6PRFJehnb1BbCNtLww