Revert "PR-B: Unify frontend URL structure and make API timeout configurable"#48
Open
IzBrain67 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reverts the earlier changes from #47 that attempted to unify frontend URL structure and make the frontend API timeout configurable, restoring prior dev/proxy and configuration behavior.
Changes:
- Removes Vite dev-server proxy routes for
/jupyter,/mcp, and/auth, and simplifies the docker Vite proxy config. - Reverts frontend API timeout handling from env-configurable to fixed per-environment values.
- Adjusts frontend env template guidance and updates Docker Compose defaults (notably
JUPYTERHUB_BASE_URL).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| gui/workflow_frontend/vite.config.ts | Removes non-/api dev proxy routes. |
| gui/workflow_frontend/vite.config.docker.ts | Hardcodes docker dev proxy target to the backend service. |
| gui/workflow_frontend/src/api/config.ts | Replaces env-parsed timeout with fixed timeouts by mode. |
| gui/workflow_frontend/env.template | Updates URL guidance and defaults (notably Jupyter base URL). |
| gui/docker-compose.yml | Updates JupyterHub base URL defaults and frontend networking/depends_on. |
Comments suppressed due to low confidence (1)
gui/workflow_frontend/vite.config.docker.ts:34
vite.config.docker.tsnow hardcodes the backend proxy target tohttp://backend:3000and no longer supportsVITE_PROXY_BACKENDoverrides. This makes docker-based dev less configurable (and is inconsistent withenv.template, which still suggests proxy configuration). Consider restoring env-based targets or updating the template/docs so users aren’t misled.
server: {
watch: {
usePolling: true,
},
proxy: {
"/api": {
target: "http://backend:3000",
changeOrigin: true,
secure: false,
},
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - ./workflow_frontend/.env | ||
| depends_on: | ||
| - backend | ||
| - backend |
| @@ -31,22 +31,6 @@ export default defineConfig({ | |||
| changeOrigin: true, | |||
| secure: false, | |||
| }, | |||
| # Browser-facing base URLs. | ||
| # These are always relative paths. In dev, the Vite proxy forwards each | ||
| # prefix to the appropriate container; in prod, nginx does the same routing. | ||
| # Behind nginx these become relative paths; in local dev they use localhost:PORT. |
Comment on lines
7
to
+16
| export const getApiConfig = (): ApiConfig => { | ||
| const isDev = (import.meta.env.MODE || "development") === "development"; | ||
| return { | ||
| baseURL: import.meta.env.VITE_API_BASE_URL, | ||
| internalSecret: | ||
| import.meta.env.VITE_INTERNAL_SECRET || (isDev ? "dev-secret" : ""), | ||
| timeout: parseTimeout(import.meta.env.VITE_API_TIMEOUT), | ||
| }; | ||
| const env = import.meta.env.MODE || "development"; | ||
|
|
||
| switch (env) { | ||
| case "production": | ||
| return { | ||
| baseURL: import.meta.env.VITE_API_BASE_URL, | ||
| internalSecret: import.meta.env.VITE_INTERNAL_SECRET || "", | ||
| timeout: 10000, | ||
| }; |
Collaborator
|
@IzBrain67 @kirillmitrofanov Thank you for your help. I will review the code myself for some days to get up to date. After that I will review future paths or collaboration or work specification with leaders before reaching you. Hold on. Thank you. |
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.
Reverts #47