Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update http-proxy-middleware #6728

Merged
merged 2 commits into from
Mar 11, 2025
Merged

Update http-proxy-middleware #6728

merged 2 commits into from
Mar 11, 2025

Conversation

timtmok
Copy link
Contributor

@timtmok timtmok commented Mar 11, 2025

Address #6075

Getting the latest http-proxy-middleware updates it and several other packages. The major version change does require updating how the proxy is constructed since the signature changed.

The legacy constructor is used but with the new options format. Using the new constructor doesn't seem to work but we need these updates.

This was an earlier test run with the new constructor and using the legacy adapter: https://github.com/posit-dev/positron/actions?query=branch%3Afeature%2Fupdate-positron-proxy++

Run 208 uses the legacy adapter and passes. Run 217 uses the new constructor and fails all the viewer and help pane tests.

@:apps @:viewer @:help @:web

Release Notes

Updates http-proxy-middleware and other packages.

New Features

  • N/A

Bug Fixes

  • N/A

QA Notes

This will affect the Viewer and Help panes when running HTML plots, apps, and help documentation.

Update setting options on http-proxy-middleware
Use legacy adapter for http-proxy-middleware
@timtmok timtmok requested a review from sharon-wang March 11, 2025 13:57
Copy link

github-actions bot commented Mar 11, 2025

E2E Tests 🚀
This PR will run tests tagged with: @:critical @:apps @:viewer @:help @:web

readme  valid tags

@timtmok
Copy link
Contributor Author

timtmok commented Mar 11, 2025

Tests to run:
@:apps @:viewer @:help

@timtmok timtmok changed the title Update Update http-proxy-middleware Mar 11, 2025
sharon-wang
sharon-wang previously approved these changes Mar 11, 2025
Copy link
Member

@sharon-wang sharon-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Help, plots and Python apps are working for me on Mac in dev builds of Positron Server Web and Positron Desktop.

I couldn't get the FastAPI Python app to work via the App Launcher button, but I'm having the same problem on 2025.03.0 build 116, so seems unrelated. If you can reproduce also, I'll open a separate issue for that! For me, a proxy for the app would start up successfully, but the app wouldn't get launched in the Terminal.

@timtmok
Copy link
Contributor Author

timtmok commented Mar 11, 2025

Adding web to the test run

@petetronic
Copy link
Collaborator

Are we separately going to update express?

@timtmok
Copy link
Contributor Author

timtmok commented Mar 11, 2025

Are we separately going to update express?

Oh yes, I meant to update it here.

@timtmok
Copy link
Contributor Author

timtmok commented Mar 11, 2025

This new change bumps the minimum version. The package-lock.json doesn't have a big diff since the first changes already updated express. I'm letting the tests run again.

Copy link
Member

@sharon-wang sharon-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once CI is green 👍

@timtmok timtmok merged commit 34a2555 into main Mar 11, 2025
8 checks passed
@timtmok timtmok deleted the feature/update-positron-proxy branch March 11, 2025 18:46
@github-actions github-actions bot locked and limited conversation to collaborators Mar 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants