feat: allow support for a root path#5904
Conversation
jordanrfrazier
requested review from
anovazzi1,
lucaseduoli and
ogabrielluiz
and removed request for
anovazzi1
github-actions
Bot
added
enhancement
ogabrielluiz
left a comment
ogabrielluiz
left a comment
There was a problem hiding this comment.
Thanks, Jordan! Tejas will appreciate this work for sure.
| // const PlaygroundPage = lazy(() => import("./pages/Playground")); | ||
|
|
||
| const SignUp = lazy(() => import("./pages/SignUpPage")); | ||
| const rootPath = process.env.LANGFLOW_ROOT_PATH || ""; |
There was a problem hiding this comment.
I think this is fixed at build time. We have an endpoint that sends config from the backend to the UI at runtime (api/v1/config).
There was a problem hiding this comment.
Do you mind explaining this a bit more? I was following the pattern I saw in the vite config - https://github.com/langflow-ai/langflow/blob/support-base-path/src/frontend/vite.config.mts#L43.
Do you mean that we should rather access the value from the backend config? If so, wouldn't that require the BE be up before a FE can come up?
There was a problem hiding this comment.
I believe that if we always set the env on the backend and pass it through a config endpoint is better than using decentralized env variables.
There was a problem hiding this comment.
The Backend serves the Frontend. Also, that variable won't be available at runtime. When the frontend is built it will grab that and fix the value to whatever it was at the build time.
There was a problem hiding this comment.
@ogabrielluiz I talked to @anovazzi1 about this and he believes that this is the correct solution. If we want the frontend to be available at a specific root path, does it not need to be available at build time?
@anovazzi1 can likely explain it better.
There was a problem hiding this comment.
What this doesn't do is allow a separate root path for the frontend and the backend, but I think that's okay for now
There was a problem hiding this comment.
I think using useGetConfig to get this puts this PR in a good state. I'd advise adding a Docker compose example on how to use/test this.
There was a problem hiding this comment.
Basically putting this on getConfig is not really useful, because this change needs to be done before the frontend builds, that way makes sense reading directly from the env instead of making a request
Could be related to this: https://fastapi.tiangolo.com/advanced/behind-a-proxy/#about-root_path Did you use uvicorn (i.e. |
Ah, yes, that must be what this is. That seems like intended behavior - do you see any issue in this then? (And I was running with |
github-actions
Bot
added
enhancement
dosubot
Bot
added
the
size:S
github-actions
Bot
added
enhancement
anovazzi1
left a comment
anovazzi1
left a comment
There was a problem hiding this comment.
I believe the root_path variable should be retrieved on the config endpoint:
|
jordanrfrazier
force-pushed
the
support-base-path
branch
from
f1b28b2 to
4daf87d
Compare
github-actions
Bot
added
enhancement
| // const PlaygroundPage = lazy(() => import("./pages/Playground")); | ||
|
|
||
| const SignUp = lazy(() => import("./pages/SignUpPage")); | ||
| const rootPath = process.env.LANGFLOW_ROOT_PATH || ""; |
There was a problem hiding this comment.
I think using useGetConfig to get this puts this PR in a good state. I'd advise adding a Docker compose example on how to use/test this.
|
Hey @jordanrfrazier, if we set the variable at build time, wouldn’t that make the Docker image less flexible, requiring users to rebuild it if they want to change the variable? |
@ogabrielluiz That's correct, but afaict, that's the only time that the routes are created (during build-time). Am I misunderstanding how those are configured? |
github-actions
Bot
added
enhancement
|
Hey @Yukiyukiyeah What's a good way to test this? |
I tested it locally with backend and frontend setup separately and change the Env Var in .env file . Since the backend route works with or without the root path, even when backend is not up and we got nothing from /config, it still works. Do we have any better way to test on cloud? I see the code @jordanrfrazier wrote before includes the same root path into the frontend route, so our frontend needs to be localhost:3030/<ROOT_PATH> . I wonder if we should keep doing that? Or we can have frontend <ROOT_PATH> different than backend? |
Ideally, I believe, we should support separate root paths. Perhaps in prod you want to serve FE requests at |
|
Hello all, |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend (Vite)
participant Backend API
Frontend (Vite)->>Backend API: GET /api/v1/config (via getBackendRootPath)
Backend API-->>Frontend (Vite): { root_path: "/langflow", ... }
Frontend (Vite)->>Frontend (Vite): Set process.env.ROOT_PATH = root_path
Frontend (Vite)->>Frontend (Vite): Use ROOT_PATH as router basename
User->>Frontend (Vite): Access app (routing uses ROOT_PATH)
sequenceDiagram
participant App
participant Settings Service
App->>Settings Service: Load settings (includes root_path)
Settings Service-->>App: Provide settings with root_path
App->>App: Use root_path for API responses and internal config
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
Bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/frontend/src/routes.tsx (1)
46-50: LGTM! Root path configuration properly implemented.The frontend correctly uses the
ROOT_PATHenvironment variable set during build time to configure the router's basename. The logging and fallback toBASENAMEare appropriate design choices.
🧹 Nitpick comments (3)
src/frontend/vite.config.mts (1)
59-59: Consider runtime vs build-time environment variable setting.Setting
ROOT_PATHas a build-time environment variable means it's baked into the bundle. This aligns with the current implementation but reduces runtime flexibility.For future enhancement, consider supporting both build-time and runtime configuration by:
- Using the current approach as the default
- Adding runtime detection that can override the build-time value
- Implementing a configuration endpoint that the frontend can call at runtime
src/frontend/src/helpers/get-backend-rootpath.ts (2)
28-37: Reconsider environment variable setting and verification.Setting
process.env.ROOT_PATHduring build time and then verifying it might be unnecessary complexity. The Vite configuration already handles this via thedefineobject.Consider simplifying by removing the environment variable setting:
- // Set the environment variable - process.env.ROOT_PATH = rootPath; - - // Verify it was set correctly - if (process.env.ROOT_PATH !== rootPath) { - console.warn( - "Failed to set ROOT_PATH environment variable, using empty root path", - ); - return ""; - }The Vite config already sets this via
"process.env.ROOT_PATH": JSON.stringify(rootPath || "").
8-8: Consider reducing console output for production builds.The console logs provide good debugging information but might be excessive for production builds.
Consider conditioning the logs based on environment:
- console.log("Fetching config from:", `${target}${configURL}`); + if (process.env.NODE_ENV !== 'production') { + console.log("Fetching config from:", `${target}${configURL}`); + }- console.log("Using root path:", rootPath); + if (process.env.NODE_ENV !== 'production') { + console.log("Using root path:", rootPath); + }Also applies to: 26-26
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.env.example(1 hunks)src/backend/base/langflow/api/v1/schemas.py(1 hunks)src/backend/base/langflow/main.py(1 hunks)src/backend/base/langflow/services/settings/base.py(1 hunks)src/frontend/src/helpers/get-backend-rootpath.ts(1 hunks)src/frontend/src/routes.tsx(2 hunks)src/frontend/vite.config.mts(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/backend/base/langflow/main.py (1)
src/backend/base/langflow/services/deps.py (1)
get_settings_service(117-130)
src/frontend/src/routes.tsx (1)
src/frontend/src/customization/config-constants.ts (1)
BASENAME(1-1)
🪛 Ruff (0.11.9)
src/backend/base/langflow/main.py
212-212: Local variable root_path is assigned to but never used
Remove assignment to unused variable root_path
(F841)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Ruff Style Check (3.13)
- GitHub Check: Optimize new Python code in this PR
- GitHub Check: Run Ruff Check and Format
- GitHub Check: Update Starter Projects
- GitHub Check: autofix
🔇 Additional comments (9)
src/backend/base/langflow/api/v1/schemas.py (1)
379-379: LGTM! Root path configuration properly exposed.The addition of
root_pathfield to theConfigResponsemodel correctly exposes the backend root path configuration through the API, allowing the frontend to dynamically fetch this value..env.example (1)
112-114: LGTM! Environment variable properly documented.The addition of
LANGFLOW_ROOT_PATHenvironment variable follows the established pattern and provides clear documentation with an example value.src/backend/base/langflow/services/settings/base.py (1)
226-227: LGTM! Root path setting properly configured.The addition of
root_pathfield to the Settings class is well-implemented with appropriate default value, type annotation, and documentation. The field will automatically read from theLANGFLOW_ROOT_PATHenvironment variable.src/frontend/src/routes.tsx (1)
197-197: Router basename correctly configured with fallback.The router basename properly uses the root path with fallback to
BASENAME, ensuring the application works correctly both with and without a custom root path.src/frontend/vite.config.mts (2)
15-16: Consider the implications of async Vite configuration.Making the Vite config async introduces a build-time dependency on the backend being available. This could cause build failures in CI/CD environments where the backend isn't running.
Consider implementing a fallback strategy or environment-based override to handle cases where the backend is unavailable during build time.
44-44: Validate the root path precedence logic.The fallback chain
rootPath || BASENAME || ""is logical, but ensure this aligns with the intended behavior whenrootPathis an empty string vs undefined.src/frontend/src/helpers/get-backend-rootpath.ts (3)
1-2: LGTM - Appropriate imports for build-time execution.Using
node-fetchis correct for the Node.js build environment, and the API helper import provides consistency with existing patterns.
17-23: Robust response validation.The type checking for
root_pathis well implemented and prevents runtime errors from invalid backend responses.
38-42: Comprehensive error handling with appropriate fallback.The catch block properly handles any unexpected errors and provides a sensible fallback behavior.
| settings = get_settings_service().settings | ||
| root_path = settings.root_path |
There was a problem hiding this comment.
Complete the root_path implementation.
The root_path is extracted from settings but never used, as flagged by static analysis. FastAPI supports a root_path parameter in its constructor to handle applications behind proxies.
Apply this diff to use the root_path in FastAPI initialization:
app = FastAPI(
title="Langflow",
version=__version__,
lifespan=lifespan,
+ root_path=root_path,
)This will enable FastAPI to properly handle requests when the application is served behind a proxy or at a custom root path.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| settings = get_settings_service().settings | |
| root_path = settings.root_path | |
| settings = get_settings_service().settings | |
| root_path = settings.root_path | |
| app = FastAPI( | |
| title="Langflow", | |
| version=__version__, | |
| lifespan=lifespan, | |
| root_path=root_path, | |
| ) |
🧰 Tools
🪛 Ruff (0.11.9)
212-212: Local variable root_path is assigned to but never used
Remove assignment to unused variable root_path
(F841)
🤖 Prompt for AI Agents
In src/backend/base/langflow/main.py around lines 211 to 212, the root_path is
retrieved from settings but not used. To fix this, pass the root_path parameter
to the FastAPI app constructor so that the application correctly handles
requests behind proxies or at custom root paths. Locate where the FastAPI app is
initialized and add root_path=root_path as an argument.
| const rootPath = await getBackendRootPath(target); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify backend availability during build process.
The build process now requires the backend to be available at build time. This could be problematic in scenarios where:
- Frontend is built before backend deployment
- CI/CD pipelines build frontend and backend separately
- Backend is not accessible from the build environment
Consider adding a timeout and more robust error handling:
- const rootPath = await getBackendRootPath(target);
+ const rootPath = await getBackendRootPath(target).catch(() => {
+ console.warn("Backend unavailable during build, using default root path");
+ return env.VITE_ROOT_PATH || BASENAME || "";
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const rootPath = await getBackendRootPath(target); | |
| const rootPath = await getBackendRootPath(target).catch(() => { | |
| console.warn("Backend unavailable during build, using default root path"); | |
| return env.VITE_ROOT_PATH || BASENAME || ""; | |
| }); |
🤖 Prompt for AI Agents
In src/frontend/vite.config.mts around lines 29 to 30, the call to
getBackendRootPath(target) assumes the backend is always available, which can
cause build failures if the backend is down or unreachable. Modify this code to
include a timeout for the backend availability check and add error handling to
catch connection failures. This will allow the build process to handle backend
unavailability gracefully, such as by retrying, logging a clear error, or
falling back to a default value.
| try { | ||
| console.log("Fetching config from:", `${target}${configURL}`); | ||
| const response = await fetch(`${target}${configURL}`); | ||
| if (!response.ok) { | ||
| console.warn( | ||
| `Failed to fetch config: ${response.status}, using empty root path`, | ||
| ); | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeout to prevent build hanging.
The fetch request lacks a timeout, which could cause the build process to hang indefinitely if the backend is slow to respond.
Add a timeout to the fetch request:
- const response = await fetch(`${target}${configURL}`);
+ const response = await fetch(`${target}${configURL}`, {
+ timeout: 5000, // 5 second timeout
+ });Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/frontend/src/helpers/get-backend-rootpath.ts around lines 7 to 15, the
fetch request does not have a timeout, which can cause the build to hang if the
backend is slow or unresponsive. Fix this by implementing a timeout mechanism
for the fetch call, such as using an AbortController to abort the fetch after a
specified timeout duration, ensuring the build process does not hang
indefinitely.
5 participants
Adds support for adding a root_path, so users can execute on a relative path.
Summary by CodeRabbit
New Features
Chores