fix : unable to load db url from env #10100
Conversation
WalkthroughIntroduces dotenv-based environment loading in SettingsService initialization, adds new SettingsService methods, adds aiomysql dependency, and creates a unit test validating environment-driven settings. Also performs a minor formatting cleanup in a CLI entrypoint. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant SettingsService
participant Env as OS Env
participant DotEnv as .env Loader
App->>SettingsService: initialize()
activate SettingsService
alt First import and .env present
SettingsService->>DotEnv: load(.env, override=false)
Note right of DotEnv: Loads variables if not already set
else Already loaded or .env missing
Note right of SettingsService: Skip dotenv load
end
SettingsService->>Env: read existing env vars
SettingsService-->>App: SettingsService(settings, auth_settings)
deactivate SettingsService
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✨ 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 |
HimavarshaVS
force-pushed
the
bug-9795
branch
2 times, most recently
from
8e5db2d to
25eabd3
Compare
HimavarshaVS
changed the title
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lfx/src/lfx/services/settings/service.py (1)
44-46: Risk of bypassing Pydantic validators with setattr.Using
setattr(self.settings, key, value)on a PydanticSettingsinstance may not reliably trigger field validators and could bypass validation logic, even withvalidate_assignment=True.Consider using direct attribute assignment or Pydantic's
model_copy(update={...})to ensure validators run:def set(self, key, value): - setattr(self.settings, key, value) - return self.settings + # Trigger Pydantic validation by direct assignment + if hasattr(self.settings, key): + setattr(self.settings, key, value) + else: + msg = f"Invalid settings key: {key}" + raise AttributeError(msg) + return self.settingsAlternatively, validate the input before assignment or document this behavior if intentional.
🧹 Nitpick comments (3)
src/lfx/src/lfx/services/settings/service.py (2)
13-19: LGTM with minor suggestions for clarity and consistency.The dotenv loading logic correctly prevents duplicate loads and respects existing environment variables (
override=False). However, consider these improvements:
Guard logging placement: The debug log "Loading .env file inside lfx settings service" is emitted even if no
.envexists. Move it inside theif env_file.exists():block to avoid misleading logs.Guard value consistency: The guard is checked with
os.environ.get("_LANGFLOW_DOTENV_LOADED")(returns string or None) and set to"1". While functional, using a consistent sentinel (e.g.,"true"or checkingin os.environ) improves readability.Apply this diff to improve logging clarity:
# Load .env file if it exists if not os.environ.get("_LANGFLOW_DOTENV_LOADED"): - logger.debug("Loading .env file inside lfx settings service") env_file = Path.cwd() / ".env" if env_file.exists(): + logger.debug("Loading .env file inside lfx settings service") load_dotenv(env_file, override=False) os.environ["_LANGFLOW_DOTENV_LOADED"] = "1"
30-42: Remove orphaned comment and verify config_dir check necessity.
Orphaned comment (Line 32): The comment "Check if a string is a valid path or a file name" is not followed by any related logic. Either remove it or complete the intended check.
config_dir validation (Lines 35-37): According to the
Settingsclass inbase.py, theset_langflow_dirvalidator sets a defaultconfig_dirusingplatformdirs.user_cache_dir. This check may be redundant unless the validator can fail or be bypassed.Apply this diff to remove the orphaned comment:
@classmethod def initialize(cls) -> SettingsService: - # Check if a string is a valid path or a file name - settings = Settings() if not settings.config_dir:If the
config_dircheck is truly defensive, add a comment explaining when it might be None despite the validator.src/backend/tests/unit/services/database/test_vertex_builds.py (1)
319-336: Test scope does not verify .env file loading.The test sets environment variables directly via
monkeypatch.setenv, which bypasses the.envfile loading mechanism introduced inservice.py(Lines 13-19). This verifies Pydantic's environment-driven settings but not the dotenv integration.To comprehensively test the PR's stated objective (loading from
.env), add a test that:
- Creates a temporary
.envfile with test variables.- Invokes
SettingsService.initialize()(which triggers dotenv loading).- Asserts settings are loaded from the file.
Example:
def test_settings_from_dotenv_file(tmp_path, monkeypatch): """Test that settings are loaded from .env file.""" # Create temp .env file env_file = tmp_path / ".env" env_file.write_text( "LANGFLOW_MAX_VERTEX_BUILDS_TO_KEEP=10\n" "LANGFLOW_DATABASE_URL=sqlite:///test.db\n" ) # Change working directory to temp path monkeypatch.chdir(tmp_path) # Reset guard to allow loading monkeypatch.delenv("_LANGFLOW_DOTENV_LOADED", raising=False) # Reload module to trigger dotenv loading import importlib import lfx.services.settings.service importlib.reload(lfx.services.settings.service) from lfx.services.settings.service import SettingsService settings_service = SettingsService.initialize() assert settings_service.settings.max_vertex_builds_to_keep == 10 assert settings_service.settings.database_url == "sqlite:///test.db"This would validate the entire .env loading path introduced in this PR.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
src/backend/base/langflow/__main__.py(0 hunks)src/backend/tests/unit/services/database/test_vertex_builds.py(1 hunks)src/lfx/pyproject.toml(1 hunks)src/lfx/src/lfx/services/settings/service.py(1 hunks)
💤 Files with no reviewable changes (1)
- src/backend/base/langflow/main.py
🧰 Additional context used
📓 Path-based instructions (5)
{src/backend/**/*.py,tests/**/*.py,Makefile}
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code
Files:
src/backend/tests/unit/services/database/test_vertex_builds.py
src/backend/tests/unit/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
Test component integration within flows using create_flow, build_flow, and get_build_events utilities
Files:
src/backend/tests/unit/services/database/test_vertex_builds.py
src/backend/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
src/backend/tests/**/*.py: Unit tests for backend code must be located in the 'src/backend/tests/' directory, with component tests organized by component subdirectory under 'src/backend/tests/unit/components/'.
Test files should use the same filename as the component under test, with an appropriate test prefix or suffix (e.g., 'my_component.py' → 'test_my_component.py').
Use the 'client' fixture (an async httpx.AsyncClient) for API tests in backend Python tests, as defined in 'src/backend/tests/conftest.py'.
When writing component tests, inherit from the appropriate base class in 'src/backend/tests/base.py' (ComponentTestBase, ComponentTestBaseWithClient, or ComponentTestBaseWithoutClient) and provide the required fixtures: 'component_class', 'default_kwargs', and 'file_names_mapping'.
Each test in backend Python test files should have a clear docstring explaining its purpose, and complex setups or mocks should be well-commented.
Test both sync and async code paths in backend Python tests, using '@pytest.mark.asyncio' for async tests.
Mock external dependencies appropriately in backend Python tests to isolate unit tests from external services.
Test error handling and edge cases in backend Python tests, including using 'pytest.raises' and asserting error messages.
Validate input/output behavior and test component initialization and configuration in backend Python tests.
Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
Be aware of ContextVar propagation in async tests; test both direct event loop execution and 'asyncio.to_thread' scenarios to ensure proper context isolation.
Test error handling by mocking internal functions using monkeypatch in backend Python tests.
Test resource cleanup in backend Python tests by using fixtures that ensure proper initialization and cleanup of resources.
Test timeout and performance constraints in backend Python tests using 'asyncio.wait_for' and timing assertions.
Test Langflow's Messag...
Files:
src/backend/tests/unit/services/database/test_vertex_builds.py
{**/test_*.py,**/*.test.{ts,tsx}}
📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)
{**/test_*.py,**/*.test.{ts,tsx}}: Warn when mocks replace testing real behavior/interactions in test files
Suggest using real objects or simpler test doubles when mocks become excessive
Ensure mocks are reserved for external dependencies, not core application logic, in tests
Test files should have descriptive test names that explain what is being validated
Organize tests logically with proper setup and teardown
Include edge cases and error conditions for comprehensive test coverage
Cover both positive and negative scenarios where appropriate
Tests must cover the main functionality being implemented
Avoid smoke-only tests; assert meaningful behavior and outcomes
Follow project testing frameworks (pytest for backend, Playwright for frontend)
For API endpoints, verify both success and error responses in tests
Files:
src/backend/tests/unit/services/database/test_vertex_builds.py
**/test_*.py
📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)
**/test_*.py: Backend test files must follow naming convention test_*.py
Backend tests should use proper pytest structure
For async Python code, use proper pytest async testing patterns (e.g., pytest-asyncio)
Files:
src/backend/tests/unit/services/database/test_vertex_builds.py
🧬 Code graph analysis (2)
src/backend/tests/unit/services/database/test_vertex_builds.py (1)
src/lfx/src/lfx/services/settings/service.py (2)
SettingsService(22-49)initialize(31-42)
src/lfx/src/lfx/services/settings/service.py (1)
src/lfx/src/lfx/services/settings/base.py (1)
Settings(60-555)
🪛 GitHub Actions: Ruff Style Check
src/backend/tests/unit/services/database/test_vertex_builds.py
[error] 336-336: ruff check failed: W292 No newline at end of file.
🪛 GitHub Check: Ruff Style Check (3.13)
src/backend/tests/unit/services/database/test_vertex_builds.py
[failure] 336-336: Ruff (W292)
src/backend/tests/unit/services/database/test_vertex_builds.py:336:56: W292 No newline at end of file
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Update Starter Projects
- GitHub Check: Optimize new Python code in this PR
🔇 Additional comments (2)
src/lfx/src/lfx/services/settings/service.py (2)
25-28: LGTM!The constructor correctly initializes the service with settings and auth settings.
48-49: LGTM!The teardown stub is appropriate for a service class and allows future cleanup logic.
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (47.08%) is below the target coverage (55.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #10100 +/- ##
=======================================
Coverage 24.17% 24.17%
=======================================
Files 1086 1086
Lines 40044 40043 -1
Branches 5541 5541
=======================================
Hits 9679 9679
+ Misses 30194 30193 -1
Partials 171 171
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
HimavarshaVS
changed the title
HimavarshaVS
changed the title
|
HimavarshaVS
changed the title
Description
Issue was raised that, despite setting LANGFLOW_DATABASE_URL in .env file, it is defaulting to sqlite.
Root cause
Pydantic settings inside lfx/settings/base.py was instantiated even before env variables are loaded when starting langflow
Fix
Make sure env variables are loaded when calling settingservice if doesn't exist already.
Testing
Before : #9795
After :
Summary by CodeRabbit
New Features
Tests
Chores
Style