-
Notifications
You must be signed in to change notification settings - Fork 8.4k
feat: Add PUT endpoint for flows upsert #11368
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
Conversation
|
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 upsert capability for flows is introduced via a PUT endpoint that handles both creation and update operations. The implementation includes folder validation, duplicate name/endpoint detection with auto-renaming on create, ownership verification, and appropriate HTTP responses (201 for create, 200 for update, 403 for unauthorized, 409 for conflicts). A helper function manages safe updates with controlled field modifications. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as PUT /{flow_id}
participant DB as Database
participant Storage as File Storage
Client->>API: upsert_flow(flow_id, flow_data)
activate API
alt Flow exists
API->>DB: Query existing flow
DB-->>API: Return Flow
API->>API: Verify ownership
alt Owner mismatch
API-->>Client: 403 Forbidden
else Owner match
API->>API: _update_existing_flow()
Note over API: Validate fs_path,<br/>Check uniqueness,<br/>Exclude id/user_id
API->>DB: Update flow record
DB-->>API: Confirm
API->>Storage: Save to filesystem
Storage-->>API: Confirm
API-->>Client: 200 OK + FlowRead
end
else Flow doesn't exist
API->>API: _new_flow(flow_id,...)
Note over API: Validate folder,<br/>Auto-rename on conflicts,<br/>Set provided flow_id
API->>DB: Create flow record
DB-->>API: Confirm + new ID
API->>Storage: Save to filesystem
Storage-->>API: Confirm
API-->>Client: 201 Created + FlowRead
end
deactivate API
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a PUT endpoint for flow upsert operations, enabling flows to be created or updated with specific IDs. This feature is designed to support syncing flows between Langflow instances while preserving their identifiers.
Changes:
- Added PUT
/api/v1/flows/{flow_id}endpoint that creates flows with specified IDs or updates existing owned flows - Enhanced
_new_flowhelper to support custom IDs and configurable conflict handling - Added comprehensive test coverage for all upsert scenarios including ownership, conflicts, and edge cases
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/backend/base/langflow/api/v1/flows.py |
Adds upsert_flow endpoint and _update_existing_flow helper; extends _new_flow with parameters for ID specification, endpoint conflict handling, and folder validation |
src/backend/tests/unit/api/v1/test_flows.py |
Adds 11 comprehensive tests covering create/update paths, ownership checks, conflict handling, folder management, and security |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (41.59%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11368 +/- ##
==========================================
+ Coverage 34.52% 34.59% +0.06%
==========================================
Files 1415 1415
Lines 67345 67401 +56
Branches 9937 9937
==========================================
+ Hits 23254 23315 +61
+ Misses 42861 42857 -4
+ Partials 1230 1229 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/backend/base/langflow/api/v1/flows.py`:
- Around line 611-613: The PUT update currently skips fs_path validation when
flow.fs_path is an empty string because the condition uses truthiness; change
the guard to explicitly check for None so empty strings are validated too (i.e.,
replace the truthy check with an explicit "is not None" style check) and call
_verify_fs_path(flow.fs_path, user_id, storage_service) in that case; locate the
check around the update handler where flow.fs_path is referenced (the block
using flow.fs_path and _verify_fs_path) and make this conditional change so
empty "" values go through _verify_fs_path validation.
🧹 Nitpick comments (1)
src/backend/base/langflow/api/v1/flows.py (1)
651-666: PUT can’t explicitly clearendpoint_name(null/empty).Line 651 drops
Nonevalues, so a client-providednullwon’t clearendpoint_name, and""would persist as a real value. Consider handling explicit null/empty similar to PATCH.Please verify the intended behavior for
endpoint_name: null/""on PUT. If clearing should be supported, a targeted override helps:♻️ Suggested handling
- update_data = flow.model_dump(exclude_unset=True, exclude_none=True) + update_data = flow.model_dump(exclude_unset=True, exclude_none=True) + if "endpoint_name" in flow.model_fields_set and (flow.endpoint_name is None or flow.endpoint_name == ""): + update_data["endpoint_name"] = None
| # Validate fs_path if provided | ||
| if flow.fs_path: | ||
| await _verify_fs_path(flow.fs_path, user_id, storage_service) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty fs_path skips validation in PUT updates.
Line 611 only validates when flow.fs_path is truthy, so "" bypasses _verify_fs_path and can persist an invalid path.
🐛 Proposed fix
- if flow.fs_path:
- await _verify_fs_path(flow.fs_path, user_id, storage_service)
+ if flow.fs_path is not None:
+ await _verify_fs_path(flow.fs_path, user_id, storage_service)🤖 Prompt for AI Agents
In `@src/backend/base/langflow/api/v1/flows.py` around lines 611 - 613, The PUT
update currently skips fs_path validation when flow.fs_path is an empty string
because the condition uses truthiness; change the guard to explicitly check for
None so empty strings are validated too (i.e., replace the truthy check with an
explicit "is not None" style check) and call _verify_fs_path(flow.fs_path,
user_id, storage_service) in that case; locate the check around the update
handler where flow.fs_path is referenced (the block using flow.fs_path and
_verify_fs_path) and make this conditional change so empty "" values go through
_verify_fs_path validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/backend/base/langflow/api/v1/flows.py`:
- Around line 611-614: The PUT path currently skips validation when flow.fs_path
is an empty string because it uses a truthy check; change the condition so that
_verify_fs_path is called whenever fs_path is provided (i.e., not only truthy)
on updates—call _verify_fs_path(flow.fs_path, user_id, storage_service) when
flow.fs_path is not None (or use hasattr/explicit check in the update handler,
e.g., in the function handling flow updates) so empty-string values are
validated and rejected consistent with create behavior.
In `@src/backend/tests/unit/api/v1/test_flows.py`:
- Around line 567-571: Assert the login response succeeded before extracting the
token: after posting login_data to "api/v1/login" and storing login_response,
add an assertion that login_response.status_code (or login_response.status)
equals the expected success code (e.g., 200) and/or check login_response.json()
contains "access_token" so the subsequent creation of other_user_headers =
{"Authorization": f"Bearer {login_response.json()['access_token']}" } is safe;
update any test helper or error message to make failures explicit.
| # Validate fs_path if provided | ||
| if flow.fs_path: | ||
| await _verify_fs_path(flow.fs_path, user_id, storage_service) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validate empty-string fs_path on PUT updates.
if flow.fs_path: skips empty strings, allowing invalid values to persist and bypassing _verify_fs_path (which already rejects ""). Consider validating whenever fs_path is provided so empty strings correctly return 400, matching create behavior.
🔧 Suggested fix
- if flow.fs_path:
- await _verify_fs_path(flow.fs_path, user_id, storage_service)
+ if flow.fs_path is not None:
+ await _verify_fs_path(flow.fs_path, user_id, storage_service)📝 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.
| # Validate fs_path if provided | |
| if flow.fs_path: | |
| await _verify_fs_path(flow.fs_path, user_id, storage_service) | |
| # Validate fs_path if provided | |
| if flow.fs_path is not None: | |
| await _verify_fs_path(flow.fs_path, user_id, storage_service) |
🤖 Prompt for AI Agents
In `@src/backend/base/langflow/api/v1/flows.py` around lines 611 - 614, The PUT
path currently skips validation when flow.fs_path is an empty string because it
uses a truthy check; change the condition so that _verify_fs_path is called
whenever fs_path is provided (i.e., not only truthy) on updates—call
_verify_fs_path(flow.fs_path, user_id, storage_service) when flow.fs_path is not
None (or use hasattr/explicit check in the update handler, e.g., in the function
handling flow updates) so empty-string values are validated and rejected
consistent with create behavior.
| # Login as other user and create a flow | ||
| login_data = {"username": "other_user_for_upsert_test", "password": "testpassword"} # pragma: allowlist secret | ||
| login_response = await client.post("api/v1/login", data=login_data) | ||
| other_user_headers = {"Authorization": f"Bearer {login_response.json()['access_token']}"} | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert login success before using the token.
If login fails, the test will raise a confusing KeyError. A clear status assertion improves signal and debugging.
🔧 Suggested fix
login_response = await client.post("api/v1/login", data=login_data)
+ assert login_response.status_code == status.HTTP_200_OK
other_user_headers = {"Authorization": f"Bearer {login_response.json()['access_token']}"}📝 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.
| # Login as other user and create a flow | |
| login_data = {"username": "other_user_for_upsert_test", "password": "testpassword"} # pragma: allowlist secret | |
| login_response = await client.post("api/v1/login", data=login_data) | |
| other_user_headers = {"Authorization": f"Bearer {login_response.json()['access_token']}"} | |
| # Login as other user and create a flow | |
| login_data = {"username": "other_user_for_upsert_test", "password": "testpassword"} # pragma: allowlist secret | |
| login_response = await client.post("api/v1/login", data=login_data) | |
| assert login_response.status_code == status.HTTP_200_OK | |
| other_user_headers = {"Authorization": f"Bearer {login_response.json()['access_token']}"} |
🤖 Prompt for AI Agents
In `@src/backend/tests/unit/api/v1/test_flows.py` around lines 567 - 571, Assert
the login response succeeded before extracting the token: after posting
login_data to "api/v1/login" and storing login_response, add an assertion that
login_response.status_code (or login_response.status) equals the expected
success code (e.g., 200) and/or check login_response.json() contains
"access_token" so the subsequent creation of other_user_headers =
{"Authorization": f"Bearer {login_response.json()['access_token']}" } is safe;
update any test helper or error message to make failures explicit.
| storage_service: Service for filesystem operations. | ||
| flow_id: Allows PUT upsert to create flows with a specific ID for syncing between instances. | ||
| fail_on_endpoint_conflict: PUT should fail predictably on conflicts rather than silently renaming. | ||
| validate_folder: PUT receives folder_id from external sources that may not exist locally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps something like: Validates the flow's folder_id exists within this project when performing a flow upsert
| if flow.user_id is None: | ||
| flow.user_id = user_id | ||
| # Validate folder_id if requested | ||
| if validate_folder and flow.folder_id is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd err on the side of failing if the flow.folder_id is None when validate_folder is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the context of the function, I agree. On the upsert case, I think if the user doesn't provide a folder id, we could just add it to the default folder. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From calling upsert function, I'd set validate_folder=False, then use the default folder. I think that means we're in agreement
| storage_service=storage_service, | ||
| flow_id=flow_id, | ||
| fail_on_endpoint_conflict=True, | ||
| validate_folder=True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(If you update new_flow to fail on empty folder_id, might need to set this to false)
| ) | ||
| ).first() | ||
| if endpoint_conflict: | ||
| raise HTTPException(status_code=409, detail="Endpoint name must be unique") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should endpoint_name uniqueness be enforced at the database layer rather than here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It already is. This is just so we can more easily catch the problem.
…ecurity, consistent JSONResponse
…attern _new_flow now handles flush, refresh, and filesystem save internally, returning FlowRead directly. This eliminates duplication between POST and PUT endpoints. PUT endpoint now uses single return with variable status code.
_new_flow now handles persistence and returns FlowRead directly, so the upload endpoint no longer needs to call flush/refresh/save.
# Conflicts: # src/lfx/src/lfx/_assets/component_index.json
| if flow.user_id is None: | ||
| flow.user_id = user_id | ||
| # Validate folder_id if requested | ||
| if validate_folder and flow.folder_id is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From calling upsert function, I'd set validate_folder=False, then use the default folder. I think that means we're in agreement
Summary
Adds a PUT endpoint (
PUT /api/v1/flows/{flow_id}) that allows creating or updating flows with a specific ID. This enables syncing flows between Langflow instances while preserving flow IDs.Behavior:
Conflict handling:
nameendpoint_namefolder_id handling:
Security:
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.