-
Notifications
You must be signed in to change notification settings - Fork 8.2k
feat: Create controller shell and schema model for new workflow API #11111
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
base: main
Are you sure you want to change the base?
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 WalkthroughThis PR introduces a new V2 Workflow API with three endpoints under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 passed)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 3
🧹 Nitpick comments (7)
src/backend/tests/unit/api/v2/test_workflow.py (2)
13-22: Good fixture design for mocking settings.The fixtures properly use context managers to scope the mock and yield the settings object. The pattern is consistent between enabled/disabled variants.
Minor note: Consider extracting the common mock setup into a helper to reduce duplication, though this is optional for test code clarity.
Also applies to: 71-80
169-213: Consider using existingflowfixture for test setup.The test creates a
Flowdirectly withsession_scope. While this works, the codebase has an existingflowfixture inconftest.pythat handles creation and cleanup. Using the existing fixture would reduce duplication and ensure consistency.However, the current implementation is functional with proper cleanup in the
finallyblock.src/lfx/src/lfx/schema/__init__.py (1)
74-113: Lazy import handlers follow established pattern.All new workflow schema classes have corresponding
__getattr__handlers that properly import from the.workflowmodule. The pattern is consistent with existing schema lazy loading.Consider consolidating these handlers using a mapping dictionary to reduce repetition:
🔎 Optional refactor suggestion
_WORKFLOW_SCHEMAS = { "WorkflowExecutionRequest", "WorkflowExecutionResponse", "WorkflowJobResponse", "WorkflowStreamEvent", "WorkflowStatusResponse", "WorkflowStopRequest", "WorkflowStopResponse", "JobStatus", "ErrorDetail", "ComponentOutput", } def __getattr__(name: str): # ... existing handlers ... if name in _WORKFLOW_SCHEMAS: from .workflow import ( ComponentOutput, ErrorDetail, JobStatus, WorkflowExecutionRequest, WorkflowExecutionResponse, WorkflowJobResponse, WorkflowStatusResponse, WorkflowStopRequest, WorkflowStopResponse, WorkflowStreamEvent, ) return locals()[name] msg = f"module '{__name__}' has no attribute '{name}'" raise AttributeError(msg)This is optional since the current implementation works correctly and matches the existing style.
src/lfx/src/lfx/schema/workflow.py (2)
11-18: LGTM - Consider documenting the FAILED vs ERROR distinction.The enum is well-structured. The distinction between
FAILEDandERRORstates could benefit from inline documentation to clarify when each applies (e.g.,FAILEDfor business logic failures vsERRORfor unexpected system errors).
139-171: LGTM - OpenAPI response definitions are well-structured.The use of
oneOffor polymorphic responses and separate content types for streaming is appropriate. Consider adding 4xx/5xx error response definitions here for comprehensive API documentation if not handled at the router level.src/backend/base/langflow/api/v2/workflow.py (2)
35-59: Clarify the purpose of thecontextparameter.The
context: dict | None = Noneparameter is unused and not injected viaDepends. If this is intended for future use, consider removing it until implemented, or if it should be a dependency, add proper injection.🔎 Proposed fix to remove unused parameter
async def execute_workflow( workflow_request: WorkflowExecutionRequest, background_tasks: BackgroundTasks, # noqa: ARG001 api_key_user: Annotated[UserRead, Depends(api_key_security)], - context: dict | None = None, # noqa: ARG001 ) -> WorkflowExecutionResponse | WorkflowJobResponse | StreamingResponse:
70-83: Consider extracting the developer API check into a reusable dependency.The
developer_api_enabledcheck is duplicated across all three endpoints. Consider creating a dependency that raises the 404 exception, reducing boilerplate and ensuring consistent behavior.🔎 Example dependency to reduce duplication
async def require_developer_api() -> None: """Dependency that ensures developer API is enabled.""" settings = get_settings_service().settings if not settings.developer_api_enabled: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="This endpoint is not available", ) # Usage in endpoints: @router.get("") async def get_workflow_status( _: Annotated[None, Depends(require_developer_api)], api_key_user: Annotated[UserRead, Depends(api_key_security)], job_id: Annotated[str, Query(description="Job ID to query")], ) -> WorkflowExecutionResponse | StreamingResponse: ...
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/backend/base/langflow/api/router.py(2 hunks)src/backend/base/langflow/api/v2/__init__.py(1 hunks)src/backend/base/langflow/api/v2/workflow.py(1 hunks)src/backend/tests/unit/api/v2/test_workflow.py(1 hunks)src/lfx/src/lfx/schema/__init__.py(2 hunks)src/lfx/src/lfx/schema/workflow.py(1 hunks)src/lfx/src/lfx/services/settings/base.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
src/backend/**/*.py: Use FastAPI async patterns withawaitfor async operations in component execution methods
Useasyncio.create_task()for background tasks and implement proper cleanup with try/except forasyncio.CancelledError
Usequeue.put_nowait()for non-blocking queue operations andasyncio.wait_for()with timeouts for controlled get operations
Files:
src/backend/tests/unit/api/v2/test_workflow.pysrc/backend/base/langflow/api/v2/__init__.pysrc/backend/base/langflow/api/v2/workflow.pysrc/backend/base/langflow/api/router.py
src/backend/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
src/backend/tests/**/*.py: Place backend unit tests insrc/backend/tests/directory, component tests insrc/backend/tests/unit/components/organized by component subdirectory, and integration tests accessible viamake integration_tests
Use same filename as component with appropriate test prefix/suffix (e.g.,my_component.py→test_my_component.py)
Use theclientfixture (FastAPI Test Client) defined insrc/backend/tests/conftest.pyfor API tests; it provides an asynchttpx.AsyncClientwith automatic in-memory SQLite database and mocked environment variables. Skip client creation by marking test with@pytest.mark.noclient
Inherit from the correctComponentTestBasefamily class located insrc/backend/tests/base.pybased on API access needs:ComponentTestBase(no API),ComponentTestBaseWithClient(needs API), orComponentTestBaseWithoutClient(pure logic). Provide three required fixtures:component_class,default_kwargs, andfile_names_mapping
Create comprehensive unit tests for all new backend components. If unit tests are incomplete, create a corresponding Markdown file documenting manual testing steps and expected outcomes
Test both sync and async code paths, mock external dependencies appropriately, test error handling and edge cases, validate input/output behavior, and test component initialization and configuration
Use@pytest.mark.asynciodecorator for async component tests and ensure async methods are properly awaited
Test background tasks usingasyncio.create_task()and verify completion withasyncio.wait_for()with appropriate timeout constraints
Test queue operations using non-blockingqueue.put_nowait()andasyncio.wait_for(queue.get(), timeout=...)to verify queue processing without blocking
Use@pytest.mark.no_blockbustermarker to skip the blockbuster plugin in specific tests
For database tests that may fail in batch runs, run them sequentially usinguv run pytest src/backend/tests/unit/test_database.pyr...
Files:
src/backend/tests/unit/api/v2/test_workflow.py
**/test_*.py
📄 CodeRabbit inference engine (Custom checks)
**/test_*.py: Review test files for excessive use of mocks that may indicate poor test design - check if tests have too many mock objects that obscure what's actually being tested
Warn when mocks are used instead of testing real behavior and interactions, and suggest using real objects or test doubles when mocks become excessive
Ensure mocks are used appropriately for external dependencies only, not for core logic
Backend test files should follow the naming convention test_*.py with proper pytest structure
Test files should have descriptive test function names that explain what is being tested
Tests should be organized logically with proper setup and teardown
Consider including edge cases and error conditions for comprehensive test coverage
Verify tests cover both positive and negative scenarios where appropriate
For async functions in backend tests, ensure proper async testing patterns are used with pytest
For API endpoints, verify both success and error response testing
Files:
src/backend/tests/unit/api/v2/test_workflow.py
src/backend/base/langflow/api/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
Backend API endpoints should be organized by version (v1/, v2/) under
src/backend/base/langflow/api/with specific modules for features (chat.py, flows.py, users.py, etc.)
Files:
src/backend/base/langflow/api/v2/__init__.pysrc/backend/base/langflow/api/v2/workflow.pysrc/backend/base/langflow/api/router.py
🧠 Learnings (9)
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Test Langflow REST API endpoints using the `client` fixture with appropriate HTTP methods (GET, POST, etc.), headers (logged_in_headers), and payload validation
Applied to files:
src/backend/tests/unit/api/v2/test_workflow.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Use predefined JSON flows and utility functions from `tests.unit.build_utils` (create_flow, build_flow, get_build_events, consume_and_assert_stream) for flow execution testing
Applied to files:
src/backend/tests/unit/api/v2/test_workflow.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Test both sync and async code paths, mock external dependencies appropriately, test error handling and edge cases, validate input/output behavior, and test component initialization and configuration
Applied to files:
src/backend/tests/unit/api/v2/test_workflow.py
📚 Learning: 2025-08-05T22:51:27.961Z
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/langflow PR: 0
File: :0-0
Timestamp: 2025-08-05T22:51:27.961Z
Learning: The TestComposioComponentAuth test in src/backend/tests/unit/components/bundles/composio/test_base_composio.py demonstrates proper integration testing patterns for external API components, including real API calls with mocking for OAuth completion, comprehensive resource cleanup, and proper environment variable handling with pytest.skip() fallbacks.
Applied to files:
src/backend/tests/unit/api/v2/test_workflow.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Use `pytest.mark.api_key_required` and `pytest.mark.no_blockbuster` markers for components that need external APIs; use `MockLanguageModel` from `tests.unit.mock_language_model` for testing without external API keys
Applied to files:
src/backend/tests/unit/api/v2/test_workflow.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Test webhook endpoints by posting to `api/v1/webhook/{endpoint_name}` with appropriate payloads and validating response status codes
Applied to files:
src/backend/tests/unit/api/v2/test_workflow.py
📚 Learning: 2025-12-03T18:17:26.561Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-12-03T18:17:26.561Z
Learning: Applies to **/test_*.py : For API endpoints, verify both success and error response testing
Applied to files:
src/backend/tests/unit/api/v2/test_workflow.py
📚 Learning: 2025-11-24T19:46:09.104Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/backend_development.mdc:0-0
Timestamp: 2025-11-24T19:46:09.104Z
Learning: Applies to src/backend/base/langflow/components/**/__init__.py : Update `__init__.py` with alphabetically sorted imports when adding new components
Applied to files:
src/backend/base/langflow/api/v2/__init__.py
📚 Learning: 2025-11-24T19:46:09.104Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/backend_development.mdc:0-0
Timestamp: 2025-11-24T19:46:09.104Z
Learning: Applies to src/backend/base/langflow/api/**/*.py : Backend API endpoints should be organized by version (v1/, v2/) under `src/backend/base/langflow/api/` with specific modules for features (chat.py, flows.py, users.py, etc.)
Applied to files:
src/backend/base/langflow/api/v2/__init__.pysrc/backend/base/langflow/api/v2/workflow.pysrc/backend/base/langflow/api/router.py
🧬 Code graph analysis (1)
src/backend/tests/unit/api/v2/test_workflow.py (2)
src/backend/tests/conftest.py (2)
flow(549-569)created_api_key(662-686)src/backend/base/langflow/services/database/models/flow/model.py (1)
Flow(186-213)
🔇 Additional comments (11)
src/backend/base/langflow/api/router.py (1)
30-30: LGTM!The workflow router import and inclusion follow the established pattern for v2 API routers. The import is correctly placed alongside other v2 imports, and the router inclusion maintains consistency with existing v2 routers.
Also applies to: 65-65
src/lfx/src/lfx/services/settings/base.py (1)
316-318: LGTM!The
developer_api_enabledsetting follows best practices:
- Secure default (
False) ensures endpoints are disabled unless explicitly enabled- Clear naming and documentation
- Consistent with existing feature flags pattern
The setting can be configured via
LANGFLOW_DEVELOPER_API_ENABLEDenvironment variable due to the class'senv_prefix="LANGFLOW_"configuration.src/backend/base/langflow/api/v2/__init__.py (1)
1-8: LGTM!The changes follow best practices:
- Module docstring added for clarity
- Relative imports are appropriate for internal package organization
__all__is alphabetically sorted per coding guidelines- Export pattern is consistent with existing routers
src/backend/tests/unit/api/v2/test_workflow.py (2)
256-288: Comprehensive authentication testing.Good coverage of API key authentication requirements across all three endpoints. The tests correctly verify the 403 status code and error message format.
Note: These tests don't mock
developer_api_enabledbecause authentication is checked before the developer API guard, which is the correct behavior to test.
215-254: Reasonable test coverage for stub endpoints.The "job exists" tests (lines 215-254) have similar logic to the "job not found" tests because the endpoints return 501 regardless of job existence. This is appropriate for stub coverage and documents expected behavior when implementation is added.
src/lfx/src/lfx/schema/__init__.py (1)
3-25: LGTM!The
__all__exports are alphabetically sorted and all new workflow schema classes are properly included. This maintains consistency with existing export conventions.src/lfx/src/lfx/schema/workflow.py (4)
21-44: LGTM!These models are well-structured with appropriate field types and descriptions.
47-89: LGTM!The request model is well-designed with
extra="forbid"for strict validation and comprehensive examples injson_schema_extrafor API documentation.
115-121: LGTM!The streaming event model is appropriately structured for SSE.
124-136: LGTM!The stop request/response models are well-designed. Using a separate
Literaltype for stop statuses rather than reusingJobStatusis appropriate since they represent different state semantics.src/backend/base/langflow/api/v2/workflow.py (1)
1-24: LGTM!The imports are well-organized and the router configuration follows FastAPI conventions. The file is correctly placed under
api/v2/as per the backend organization guidelines.
- Add workflow API endpoints (POST /workflow, GET /workflow, POST /workflow/stop) - Implement developer API protection with settings check - Add comprehensive workflow schema models with proper validation - Create extensive unit test suite covering all scenarios - Apply Ruff linting standards and fix all code quality issues - Support API key authentication for all workflow endpoints
677583f to
91d06d4
Compare
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.