skip Positron Proxy for fastapi and streamlit apps on PWB #5138

sharon-wang · 2024-10-23T18:32:00Z

addresses: App Frameworks on Workbench: Fastapi root path issues #5027 and App Frameworks on Workbench: Streamlit websocket issue when SSL enabled #5026
skip the Positron Proxy when running fastapi and streamlit apps in Positron on Workbench
add log output channel for Positron Proxy for easier debugging
add typescript dev dependency to Positron Run App package.json to make vsce package available for that extension

QA Notes

I've been testing with the apps from https://github.com/posit-dev/qa-example-content/tree/main/workspaces/python_apps.

Positron on Workbench (Linux Ubuntu x86_64): Dash, Fastapi, Flask, Gradio, Streamlit all good ✅
- Note that I sometimes have to refresh the Viewer for the Fastapi app before it works, as the app may be opened in the Viewer before the app is actually ready
Positron Server Web (Mac): Dash, Fastapi, Flask, Gradio, Streamlit all good ✅
Positron Desktop (Mac): Dash, Fastapi, Flask, Gradio, Streamlit all good ✅

I thought it was fixing something, but it turns out it's not, and actually it breaks some frameworks such as Dash.

sharon-wang · 2024-10-23T18:34:51Z

extensions/positron-run-app/src/api.ts


 	// Preview the external URI.
-	positron.window.previewUrl(proxyInfo.externalUri);
+	positron.window.previewUrl(previewUri);


@seeM When running Fastapi in Positron on Workbench, sometimes the app url is previewed in the Viewer slightly too early, before the app server is ready. To get around this, a user can refresh the Viewer, but that is not ideal if it happens every time.

Do you have thoughts on how to get around this issue? Maybe we can ping the uri to check if it's responsive before displaying it in the Viewer?

Maybe #4276 is describing what I'm seeing?

I assume that uvicorn would only print the URL to stdout once the server is ready to receive connections (I haven't confirmed that). Is the delay happening at the Positron Proxy layer? If so, maybe there's something we could expose from the Positron Proxy extension i.e. an awaitable or an event that we could listen to here?

I looked into this a bit more and it does not seem to be in the Positron Proxy layer, as the issue still occurs even if we don't use the Positron Proxy.

For fastapi with uvicorn, we may want to wait for Application startup complete in the terminal output before we open the app at the url. We happen to be just a little quick to open the url/slow to startup the app on Workbench, such that the fastapi app is not quite ready yet.

Perhaps this would be a separate property in the terminal/debug config for some terminal text that indicates the app is ready, before we extract the app url and view it. I'll open a separate issue for this.

Opened #5187

extensions/positron-run-app/src/api.ts

seeM

I haven't setup a workbench dev environment so I haven't tested the changes, but code changes look good. We should try to fix the fastapi preview if possible but don't think it has to block this PR.

extensions/positron-run-app/src/api.ts

seeM · 2024-10-25T12:55:35Z

extensions/positron-run-app/src/api.ts

-	log.debug(`Viewing app at local uri: ${localUri} with external uri ${proxyInfo.externalUri.toString()}`);
+	// Determine the target origin based on whether the server is running in PWB.
+	const runningInPWB = process.env.RS_SERVER_URL ? true : false;
+	const targetOrigin = runningInPWB ? localBaseUri : localUri;


Interesting. I'm curious why the target origin differs here for workbench

It's a bit of a hack as I couldn't get UVICORN_ROOT_PATH or --root-path to work for fastapi in Positron Server Web or Desktop. I haven't gotten to the bottom of why it's working on Workbench.

The result on Positron Server Web without this handling is that the openapi.json file is getting loaded from the root /openapi.json instead of from e.g. proxy/1234/openapi.json, so we get:

I'm gonna move this PR to Draft while I try a few more things to see if we can avoid this custom handling.

Positron Server Web still needs this workaround. I still don't understand why the root path setting doesn't work.

I've decided to not pass fastapi through the Positron Proxy for now. I'll probably do the same for Streamlit as well.

seeM · 2024-10-25T12:58:26Z

extensions/positron-run-app/src/api.ts


 	// Preview the external URI.
-	positron.window.previewUrl(proxyInfo.externalUri);
+	positron.window.previewUrl(previewUri);


I assume that uvicorn would only print the URL to stdout once the server is ready to receive connections (I haven't confirmed that). Is the delay happening at the Positron Proxy layer? If so, maybe there's something we could expose from the Positron Proxy extension i.e. an awaitable or an event that we could listen to here?

this allows us to run `vsce package` in the `extensions/positron-run-app` directory to generate a VSIX which can be loaded into Positron for faster extension dev iteration when running a built reh-web version of Positron.

sharon-wang · 2024-10-28T21:33:56Z

extensions/positron-run-app/package.json

+    "@types/sinon-test": "^2.4.6",
+    "typescript": "^4.5.5"


this allows us to run vsce package in the extensions/positron-run-app directory to generate a VSIX, which can be loaded into Positron. This is helpful for faster extension dev iteration when running a pre-built reh-web version of Positron.

proxying kind of works, but getting: Refused to apply style from 'path/to/style.css' because its MIME type ('text/html') is not a supported stylesheet MIME type, and strict MIME checking is enabled.

This reverts commit 07b544b.

This reverts commit 7e15c26.

This reverts commit aeab1f2.

This reverts commit 2e9b4ad.

This reverts commit e4fa27e.

sharon-wang and others added 5 commits October 23, 2024 09:24

add log output channel for positron proxy

f260290

fix handling of custom urlPath in positron-run-app

7cc22a9

remove trailing slashes and add trace logging

aaf999b

add workbench-specific handling to determine the proxy targetOrigin

a5d3c2f

remove removal of trailing slash from target origin

c5aca3c

I thought it was fixing something, but it turns out it's not, and actually it breaks some frameworks such as Dash.

sharon-wang requested review from seeM and softwarenerd October 23, 2024 18:32

sharon-wang commented Oct 23, 2024

View reviewed changes

sharon-wang mentioned this pull request Oct 23, 2024

App Frameworks on Workbench: Fastapi root path issues #5027

Closed

sharon-wang commented Oct 23, 2024

View reviewed changes

extensions/positron-run-app/src/api.ts Outdated Show resolved Hide resolved

seeM previously approved these changes Oct 25, 2024

View reviewed changes

remove fastapi workbench-specific workaround

e817274

sharon-wang marked this pull request as draft October 28, 2024 17:05

sharon-wang added 2 commits October 28, 2024 13:33

skip positron proxy for fastapi apps on PWB

ec92a71

remove fastapi workbench-specific workaround

8cf5740

sharon-wang dismissed seeM’s stale review via 8cf5740 October 28, 2024 20:40

update function doc and match style with other functions

9cea3be

sharon-wang mentioned this pull request Oct 28, 2024

Run App: app may be opened in Viewer before it's ready #5187

Closed

sharon-wang marked this pull request as ready for review October 28, 2024 21:30

sharon-wang requested a review from seeM October 28, 2024 21:30

add typescript dep to positron-run-app

88696b2

this allows us to run `vsce package` in the `extensions/positron-run-app` directory to generate a VSIX which can be loaded into Positron for faster extension dev iteration when running a built reh-web version of Positron.

sharon-wang commented Oct 28, 2024

View reviewed changes

sharon-wang and others added 8 commits October 29, 2024 10:32

add websocket proxy req logging and explicit upgrade handling

1528565

override websocket proxy request origin header

a2b51ae

WIP: extra handling in positron proxy for websockets

b91c7f5

add debug logging for websocket upgrades

b67a1b5

try to rewrite the proxy with http-proxy

c8ce6a1

proxying kind of works, but getting: Refused to apply style from 'path/to/style.css' because its MIME type ('text/html') is not a supported stylesheet MIME type, and strict MIME checking is enabled.

Revert "try to rewrite the proxy with http-proxy"

d142b12

This reverts commit 07b544b.

Revert "add debug logging for websocket upgrades"

df57747

This reverts commit 7e15c26.

Revert "WIP: extra handling in positron proxy for websockets"

48d38f8

This reverts commit aeab1f2.

sharon-wang added 3 commits October 29, 2024 10:32

Revert "override websocket proxy request origin header"

8f4713d

This reverts commit 2e9b4ad.

Revert "add websocket proxy req logging and explicit upgrade handling"

465e2f0

This reverts commit e4fa27e.

disable positron proxy for streamlit on PWB

d4fc66d

sharon-wang changed the title ~~Fix issues running fastapi apps on PWB~~ Fix issues running fastapi and streamlit apps on PWB Oct 29, 2024

sharon-wang changed the title ~~Fix issues running fastapi and streamlit apps on PWB~~ skip Positron Proxy for fastapi and streamlit apps on PWB Oct 29, 2024

seeM approved these changes Oct 29, 2024

View reviewed changes

sharon-wang merged commit a896e6e into main Oct 29, 2024
3 checks passed

sharon-wang deleted the fix-fastapi-on-workbench branch October 29, 2024 16:21

github-actions bot locked and limited conversation to collaborators Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

skip Positron Proxy for fastapi and streamlit apps on PWB #5138

skip Positron Proxy for fastapi and streamlit apps on PWB #5138

sharon-wang commented Oct 23, 2024 •

edited

Loading

sharon-wang Oct 23, 2024

sharon-wang Oct 23, 2024

seeM Oct 25, 2024

sharon-wang Oct 28, 2024

sharon-wang Oct 28, 2024

seeM left a comment

seeM Oct 25, 2024

sharon-wang Oct 28, 2024

sharon-wang Oct 28, 2024

sharon-wang Oct 28, 2024 •

edited

Loading

seeM Oct 25, 2024

sharon-wang Oct 28, 2024

skip Positron Proxy for fastapi and streamlit apps on PWB #5138

skip Positron Proxy for fastapi and streamlit apps on PWB #5138

Conversation

sharon-wang commented Oct 23, 2024 • edited Loading

QA Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seeM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharon-wang Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharon-wang commented Oct 23, 2024 •

edited

Loading

sharon-wang Oct 28, 2024 •

edited

Loading