Use fastapi run for FastAPI debug configs, with file and project variants#1048
Use fastapi run for FastAPI debug configs, with file and project variants#1048savannahostrowski wants to merge 9 commits into
fastapi run for FastAPI debug configs, with file and project variants#1048Conversation
|
/azp run |
|
@microsoft-github-policy-service agree [company="FastAPI Labs"] |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
@microsoft-github-policy-service agree company="FastAPI Labs" |
|
So just closing the loop here and reconciling threads from #1045, since the PR only changes what the snippet flow writes when a user newly invokes "Add Configuration" and what the dynamic provider suggests in-memory for users without a launch.json, anyone with module: uvicorn already in their launch.json keeps that exact config and continues to work. I've also added tiered auto-detection so monorepo/nested layouts don't break out of the box: Auto-detection (applied in both flows):
The snippet flow also prompts on 0 matches (with no pre-fill). The dynamic provider doesn't surface FastAPI configs at all if 0 are detected. |
| return fastApiPaths; | ||
| } | ||
|
|
||
| export function tryResolveFastApiArgs(folder: WorkspaceFolder, paths: Uri[]): string[] | undefined { |
There was a problem hiding this comment.
Copilot generated:
tryResolveFastApiArgs emits OS-specific separators into args via path.relative (e.g. backend\app\main.py on Windows). A launch.json generated on Windows won't resolve when shared to macOS/Linux. Consider normalizing to POSIX separators in the persisted arg. [unverified] whether fastapi run normalizes backslashes on Windows. Note: the Django provider has the same pre-existing pattern, so this is not a new convention break.
[verified]
There was a problem hiding this comment.
Django and Flask in the dynamic provider have used path.relative directly (without normalization) on main for a long time. To stay consistent with that existing convention, I left this as-is. Let me know if that's an issue.
| export async function buildFastAPIWithFileLaunchDebugConfiguration( | ||
| _input: MultiStepInput<DebugConfigurationState>, | ||
| state: DebugConfigurationState, | ||
| ): Promise<void> { |
There was a problem hiding this comment.
Copilot generated:
The Architect notes the ['run', '${file}'] config literal now lives in two places (here and dynamicdebugConfigurationService.ts), and the ['run'] default is encoded both in tryResolveFastApiArgs and as ?? ['run'] in the dynamic caller. Hoist to a shared constant / small helper to prevent drift. Low priority.
[verified]
There was a problem hiding this comment.
I tried this but extracting ['run', '${file}'] into a readonly string[] constant required spreading at every call site ([...FASTAPI_RUN_FILE_ARGS]), which was longer than the inline literal.
| return fastApiPaths; | ||
| } | ||
|
|
||
| export function tryResolveFastApiArgs(folder: WorkspaceFolder, paths: readonly Uri[]): string[] | undefined { |
There was a problem hiding this comment.
Copilot generated:
Unresolved from prior review (Medium): path.relative emits OS-native separators, so a single subdir match persists "args": ["run", "backend\\app\\main.py"] on Windows. A launch.json shared to macOS/Linux then runs fastapi run backend\app\main.py, which treats backslashes literally and fails despite correct detection. The Skeptic notes the central fix actually broadened exposure here (root single-matches previously emitted separator-free ['run']). Normalize to POSIX in the persisted arg (e.g. relative.split(path.sep).join('/')) and confirm fastapi run accepts forward slashes on Windows.
[verified]
| const appPyPath = path.join(folder.uri.fsPath, 'main.py'); | ||
| pathExistsStub.withArgs(appPyPath).resolves(false); | ||
| const file = await fastApiLaunch.getApplicationPath(folder); | ||
| const state = { config: {}, folder }; |
There was a problem hiding this comment.
Copilot generated:
The Skeptic notes the nested-match expectations use path.join('backend','app','main.py'), which produces the resolver's own OS-specific output on every platform — the separator assertion is tautological and will never catch the portability bug above. Once the persisted arg is normalized to POSIX, assert the literal 'backend/app/main.py' instead.
[verified]
2 participants
A draft for #1045 - opening this to make the proposed approach a bit more concrete. Happy to adjust based on the open discussion before marking it review ready (there are some open questions to align on in the issue).
This updates the FastAPI debug configuration (both the
launch.jsonsnippet and the dynamic provider) to use thefastapiCLI instead of invokinguvicorndirectly:Debug FastAPI with current file - runs
fastapi run ${file}against the currently active file. This is optimal for single-file apps or quickly debugging a specific entry point.Debug FastAPI - runs plain
fastapi runand lets the CLI discover the app viapyproject.tomlor default file locations.This replaces the previous
uvicorn main:app --reloadconfig and its hand-rolledmain.pydetection.I have run the tests and manually tested as well.