Fix: Reload context on config change (#2269)

mykalmax · web-flow · commit 0384db3ac42c · 2024-03-18T13:52:52.000-07:00
diff --git a/sqlmesh/core/loader.py b/sqlmesh/core/loader.py
@@ -156,7 +156,7 @@ def reload_needed(self) -> bool:
             True if a modification is found; False otherwise
         """
         return any(
-            path.stat().st_mtime > initial_mtime
+            not path.exists() or path.stat().st_mtime > initial_mtime
             for path, initial_mtime in self._path_mtimes.items()
         )
 
diff --git a/web/client/src/library/components/documentation/Documentation.tsx b/web/client/src/library/components/documentation/Documentation.tsx
@@ -47,7 +47,7 @@ const Documentation = function Documentation({
     return () => {
       cancelRequestModel()
     }
-  }, [model.hash])
+  }, [model.name, model.hash])
 
   return (
     <Container>
diff --git a/web/client/src/library/components/editor/EditorInspector.tsx b/web/client/src/library/components/editor/EditorInspector.tsx
@@ -584,7 +584,7 @@ function FormDiffModel({
     void getDiff().then(({ data }) => {
       setPreviewDiff(data)
     })
-  }, [model.hash])
+  }, [model.name, model.hash])
 
   useEffect(() => {
     return () => {
diff --git a/web/client/src/library/components/fileExplorer/Directory.tsx b/web/client/src/library/components/fileExplorer/Directory.tsx
@@ -32,7 +32,7 @@ const Directory = function Directory({
   const [isDraggable, setIsDraggable] = useState(false)
 
   const attrs = useLongPress(() => setIsDraggable(true), {
-    threshold: 500,
+    threshold: 50,
     onFinish() {
       setIsDraggable(false)
     },
diff --git a/web/client/src/library/components/fileExplorer/File.tsx b/web/client/src/library/components/fileExplorer/File.tsx
@@ -35,7 +35,7 @@ function File({
   const [isDraggable, setIsDraggable] = useState(false)
 
   const attrs = useLongPress(() => setIsDraggable(true), {
-    threshold: 500,
+    threshold: 50,
     onFinish() {
       setIsDraggable(false)
     },
diff --git a/web/client/src/library/components/graph/ModelLineage.tsx b/web/client/src/library/components/graph/ModelLineage.tsx
@@ -118,7 +118,7 @@ export default function ModelLineage({
       setMainNode(undefined)
       setHighlightedNodes({})
     }
-  }, [model.hash])
+  }, [model.name, model.hash])
 
   useEffect(() => {
     Object.keys(modelLineage ?? {}).forEach(modelName => {
diff --git a/web/client/src/library/components/loading/LoadingStatus.tsx b/web/client/src/library/components/loading/LoadingStatus.tsx
@@ -9,7 +9,7 @@ export default function LoadingStatus({
   return (
     <Loading className="inline-block">
       <Spinner className="w-3 h-3 border border-neutral-10 mr-2" />
-      <span className="inline-block text-xs">{children}</span>
+      <span className="inline-block text-xs whitespace-nowrap">{children}</span>
     </Loading>
   )
 }
diff --git a/web/client/src/library/pages/root/Navigation.tsx b/web/client/src/library/pages/root/Navigation.tsx
@@ -14,16 +14,17 @@ export default function PageNavigation(): JSX.Element {
   return (
     <div
       className={clsx(
-        'min-w-[10rem] px-2 min-h-8 max-h-8 w-full flex items-center',
+        'relative min-w-[10rem] px-2 min-h-8 max-h-8 w-full flex items-center',
         modules.showHistoryNavigation ? 'justify-between' : 'justify-end',
       )}
     >
       {modules.showHistoryNavigation && <HistoryNavigation />}
-      {isFetchingModels ? (
-        <LoadingStatus>Loading Models...</LoadingStatus>
-      ) : (
-        modules.hasPlans && <EnvironmentDetails />
+      {isFetchingModels && (
+        <div className="absolute w-full h-full flex justify-center items-center z-10 bg-transparent-20 backdrop-blur-lg">
+          <LoadingStatus>Loading Models...</LoadingStatus>
+        </div>
       )}
+      {modules.hasPlans && <EnvironmentDetails />}
       {modules.hasErrors && <ReportErrors />}
     </div>
   )
diff --git a/web/server/api/endpoints/directories.py b/web/server/api/endpoints/directories.py
@@ -5,26 +5,23 @@
 from fastapi import APIRouter, Body, Depends, HTTPException, Response, status
 from starlette.status import HTTP_404_NOT_FOUND
 
-from sqlmesh.core.context import Context
 from web.server import models
 from web.server.exceptions import ApiException
-from web.server.settings import Settings, get_context, get_settings
+from web.server.settings import Settings, get_settings
 from web.server.utils import replace_file, validate_path
 
 router = APIRouter()
 
 
 @router.post("/{path:path}", response_model=models.Directory)
 async def write_directory(
-    response: Response,
     path: str = Depends(validate_path),
     new_path: t.Optional[str] = Body(None, embed=True),
     settings: Settings = Depends(get_settings),
-    context: Context = Depends(get_context),
 ) -> models.Directory:
     """Create or rename a directory."""
     if new_path:
-        validate_path(new_path, context)
+        new_path = await validate_path(new_path, settings)
         replace_file(settings.project_path / path, settings.project_path / new_path)
         return models.Directory(name=os.path.basename(new_path), path=new_path)
 
diff --git a/web/server/api/endpoints/files.py b/web/server/api/endpoints/files.py
@@ -25,16 +25,9 @@
 
 
 @router.get("", response_model=models.Directory)
-def get_files(
-    context: t.Optional[Context] = Depends(get_context),
-    settings: Settings = Depends(get_settings),
-) -> models.Directory:
+async def get_files(settings: Settings = Depends(get_settings)) -> models.Directory:
     """Get all project files."""
-    return _get_directory(
-        path=settings.project_path,
-        settings=settings,
-        context=context,
-    )
+    return await _get_directory(settings.project_path, settings)
 
 
 @router.get("/{path:path}", response_model=models.File)
@@ -54,21 +47,26 @@ def get_file(
 @router.post("/{path:path}", response_model=t.Optional[models.File])
 async def write_file(
     response: Response,
+    path: str = Depends(validate_path),
     content: str = Body("", embed=True),
     new_path: t.Optional[str] = Body(None, embed=True),
-    path: str = Depends(validate_path),
     settings: Settings = Depends(get_settings),
-    context: Context = Depends(get_context),
+    context: t.Optional[Context] = Depends(get_context),
 ) -> t.Optional[models.File]:
     """Create, update, or rename a file."""
     path_or_new_path = path
     if new_path:
-        path_or_new_path = validate_path(new_path, context)
+        path_or_new_path = await validate_path(new_path, settings)
         replace_file(settings.project_path / path, settings.project_path / path_or_new_path)
     else:
         full_path = settings.project_path / path
-        config = context.config_for_path(Path(path_or_new_path))
-        if config.ui.format_on_save and content and Path(path_or_new_path).suffix == ".sql":
+        config = context.config_for_path(Path(path_or_new_path)) if context else None
+        if (
+            config
+            and config.ui.format_on_save
+            and content
+            and Path(path_or_new_path).suffix == ".sql"
+        ):
             format_file_status = models.FormatFileStatus(
                 status=models.Status.INIT, path=path_or_new_path
             )
@@ -120,11 +118,8 @@ async def delete_file(
         )
 
 
-def _get_directory(
-    path: str | Path,
-    settings: Settings,
-    context: t.Optional[Context] = None,
-) -> models.Directory:
+async def _get_directory(path: str | Path, settings: Settings) -> models.Directory:
+    context = await get_context(settings)
     ignore_patterns = context.config.ignore_patterns if context else c.IGNORE_PATTERNS
 
     def walk_path(
diff --git a/web/server/settings.py b/web/server/settings.py
@@ -125,3 +125,9 @@ async def get_context_or_raise(settings: Settings = Depends(get_settings)) -> Co
             origin="API -> settings -> get_context",
         )
     return context
+
+
+def invalidate_context_cache() -> None:
+    _get_context.cache_clear()
+    _get_loaded_context.cache_clear()
+    _get_path_to_model_mapping.cache_clear()
diff --git a/web/server/utils.py b/web/server/utils.py
@@ -12,10 +12,10 @@
 from starlette.responses import StreamingResponse
 from starlette.status import HTTP_404_NOT_FOUND, HTTP_422_UNPROCESSABLE_ENTITY
 
-from sqlmesh.core.context import Context
+from sqlmesh.core import constants as c
 from web.server.console import api_console
 from web.server.exceptions import ApiException
-from web.server.settings import get_context
+from web.server.settings import Settings, get_context, get_settings
 
 R = t.TypeVar("R")
 
@@ -51,19 +51,20 @@ def func_wrapper() -> R:
     return await loop.run_in_executor(None, func_wrapper)
 
 
-def validate_path(
-    path: str,
-    context: Context = Depends(get_context),
-) -> str:
-    resolved_path = context.path.resolve()
+async def validate_path(path: str, settings: Settings = Depends(get_settings)) -> str:
+    context = await get_context(settings)
+    resolved_path = settings.project_path.resolve()
     full_path = (resolved_path / path).resolve()
     try:
         full_path.relative_to(resolved_path)
     except ValueError:
         raise HTTPException(status_code=HTTP_404_NOT_FOUND)
 
     if any(
-        full_path.match(pattern) for pattern in context.config_for_path(Path(path)).ignore_patterns
+        full_path.match(pattern)
+        for pattern in (
+            context.config_for_path(Path(path)).ignore_patterns if context else c.IGNORE_PATTERNS
+        )
     ):
         raise HTTPException(status_code=HTTP_404_NOT_FOUND)
 
diff --git a/web/server/watcher.py b/web/server/watcher.py
@@ -4,34 +4,48 @@
 from watchfiles import Change, DefaultFilter, awatch
 
 from sqlmesh.core import constants as c
+from sqlmesh.core.context import Context
 from web.server import models
 from web.server.api.endpoints.files import _get_directory, _get_file_with_content
 from web.server.console import api_console
 from web.server.exceptions import ApiException
-from web.server.settings import get_context_or_raise, get_settings
+from web.server.settings import (
+    Settings,
+    get_context,
+    get_settings,
+    invalidate_context_cache,
+)
+from web.server.utils import is_relative_to
 
 
 async def watch_project() -> None:
     settings = get_settings()
-    context = await get_context_or_raise(settings)
+    context = await get_context(settings)
+    paths = [
+        (settings.project_path / c.AUDITS).resolve(),
+        (settings.project_path / c.MACROS).resolve(),
+        (settings.project_path / c.MODELS).resolve(),
+        (settings.project_path / c.METRICS).resolve(),
+        (settings.project_path / c.SEEDS).resolve(),
+    ]
     ignore_entity_patterns = context.config.ignore_patterns if context else c.IGNORE_PATTERNS
     ignore_entity_patterns.append("^\\.DS_Store$")
     ignore_entity_patterns.append("^.*\.db(\.wal)?$")
-    ignore_paths = [str((context.path / c.CACHE).resolve())]
+    ignore_paths = [str((settings.project_path / c.CACHE).resolve())]
     watch_filter = DefaultFilter(
         ignore_paths=ignore_paths, ignore_entity_patterns=ignore_entity_patterns
     )
-
-    async for entries in awatch(context.path, watch_filter=watch_filter, force_polling=True):
+    async for entries in awatch(
+        settings.project_path, watch_filter=watch_filter, force_polling=True
+    ):
         changes: t.List[models.ArtifactChange] = []
         directories: t.Dict[str, models.Directory] = {}
         for change, path_str in entries:
             path = Path(path_str)
             try:
                 relative_path = path.relative_to(settings.project_path)
-
                 if change == Change.modified and path.is_dir():
-                    directory = _get_directory(path, settings, context)
+                    directory = await _get_directory(path, settings)
                     directories[directory.path] = directory
                 elif change == Change.deleted or not path.exists():
                     changes.append(
@@ -51,6 +65,16 @@ async def watch_project() -> None:
                             ),
                         )
                     )
+                if context:
+                    in_paths = any(is_relative_to(path, p) for p in paths)
+                    is_modified_new_file = (
+                        change == Change.modified and path not in context._loader._path_mtimes
+                    )
+                    should_track_file = path.is_file() and in_paths
+                    should_reset_mtime = Change.added or is_modified_new_file
+                    if should_track_file and should_reset_mtime:
+                        context._loader._path_mtimes[path] = 0
+
             except Exception:
                 error = ApiException(
                     message="Error updating file",
@@ -64,3 +88,26 @@ async def watch_project() -> None:
                 event=models.EventName.FILE,
                 data={"changes": changes, "directories": directories},
             )
+
+        if is_config_changed(entries, settings, context):
+            invalidate_context_cache()
+            api_console.log_event(
+                event=models.EventName.WARNINGS,
+                data=ApiException(
+                    message="Config file changed",
+                    origin="API -> watcher -> watch_project",
+                    trigger="config",
+                ).to_dict(),
+            )
+
+
+def is_config_changed(
+    entries: t.Set[t.Any], settings: Settings, context: t.Optional[Context] = None
+) -> bool:
+    config_paths = set(context.configs) if context else {settings.project_path}
+
+    return any(
+        Path(path) in config_path.glob("config.*")
+        for config_path in config_paths
+        for _, path in entries
+    )

Original file line number	Diff line number	Diff line change
`@@ -156,7 +156,7 @@ def reload_needed(self) -> bool:`
`156`	`156`	`True if a modification is found; False otherwise`
`157`	`157`	`"""`
`158`	`158`	`return any(`
`159`		`- path.stat().st_mtime > initial_mtime`
	`159`	`+ not path.exists() or path.stat().st_mtime > initial_mtime`
`160`	`160`	`for path, initial_mtime in self._path_mtimes.items()`
`161`	`161`	`)`
`162`	`162`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ const Documentation = function Documentation({`
`47`	`47`	`return () => {`
`48`	`48`	`cancelRequestModel()`
`49`	`49`	`}`
`50`		`- }, [model.hash])`
	`50`	`+ }, [model.name, model.hash])`
`51`	`51`
`52`	`52`	`return (`
`53`	`53`	`<Container>`
Original file line number	Diff line number	Diff line change
`@@ -118,7 +118,7 @@ export default function ModelLineage({`
`118`	`118`	`setMainNode(undefined)`
`119`	`119`	`setHighlightedNodes({})`
`120`	`120`	`}`
`121`		`- }, [model.hash])`
	`121`	`+ }, [model.name, model.hash])`
`122`	`122`
`123`	`123`	`useEffect(() => {`
`124`	`124`	`Object.keys(modelLineage ?? {}).forEach(modelName => {`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ export default function LoadingStatus({`
`9`	`9`	`return (`
`10`	`10`	`<Loading className="inline-block">`
`11`	`11`	`<Spinner className="w-3 h-3 border border-neutral-10 mr-2" />`
`12`		`- <span className="inline-block text-xs">{children}</span>`
	`12`	`+ <span className="inline-block text-xs whitespace-nowrap">{children}</span>`
`13`	`13`	`</Loading>`
`14`	`14`	`)`
`15`	`15`	`}`