Plan demo workspace migration to template #312
Conversation
- Add demo_workspaces configuration in settings.json with options for enabling, source directory, and auto-loading for online sessions - Create example-data/workspaces/default/ with pre-configured mzML files and params.json for immediate demo experience - Modify page_setup() to detect and copy demo workspaces: - When a demo workspace name is requested via URL, copy to new UUID workspace - Auto-load default demo for new online sessions when configured - Copy demo workspaces for local mode when workspaces are disabled - Add sidebar UI for online users to load demo data into current workspace - Add helper functions get_available_demo_workspaces() and copy_demo_workspace() This migrates the demo workspace handling pattern from FLASHApp to the template, allowing derivative apps to ship with pre-configured example data that users can explore without uploading their own files.
Users must now explicitly load demo data via the sidebar UI or access a demo workspace URL (e.g., ?workspace=default). New online sessions start with an empty workspace.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds demo workspace management to a Streamlit application. Introduces a new Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as Sidebar UI
participant Handler as Demo Handler
participant FileOps as File System
participant Workspace as Target Workspace
User->>UI: Select demo from expander
UI->>Handler: copy_demo_workspace(demo_name, target_path)
Handler->>Handler: Validate demo name (is_safe_workspace_name)
Handler->>FileOps: Find demo source directory
Handler->>FileOps: Read demo files
FileOps-->>Handler: Demo contents
Handler->>Workspace: Copy files and directories
Workspace-->>Handler: Copy complete
Handler-->>UI: Success
UI-->>User: Demo data loaded
Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/common/common.py`:
- Around line 370-379: Wrap the shutil.copytree call inside a try/except to
catch filesystem errors when copying demo workspaces: in the block that uses
demo_config, demo_source_dir, workspaces_dir and target, surround
shutil.copytree(demo, target) with a try: ... except Exception as e: ... that
logs the demo name/path and the exception (include the exception string) and
continues without raising; alternatively call an existing copy_demo_workspace()
helper if available instead of direct copytree. Ensure the log includes clear
context (which demo was being copied) so failures don't fail silently.
- Around line 344-353: The current logic updates st.session_state.workspace and
st.query_params.workspace before calling shutil.copytree, which can leave the
app in an inconsistent state if the copy fails; replace the direct
shutil.copytree call with the existing helper copy_demo_workspace() (or wrap the
copy in try/except) so the copy is attempted first and only on success set
st.session_state.workspace and st.query_params.workspace (refer to
st.session_state.location, requested_workspace, available_demos, workspace_id,
st.session_state.workspace, st.query_params.workspace, shutil.copytree and
copy_demo_workspace()) and log/handle errors to avoid leaving partial state.
🧹 Nitpick comments (4)
src/common/common.py (4)
68-74: Consider logging or preserving error context in exception handler.The broad
except Exceptionsilently swallows all errors, making it difficult to diagnose issues when demo workspace copying fails. At minimum, consider logging the exception.♻️ Suggested improvement
try: if target_path.exists(): shutil.rmtree(target_path) shutil.copytree(demo_path, target_path) return True - except Exception: + except Exception as e: + st.warning(f"Failed to copy demo workspace '{demo_name}': {e}") return False
332-338: Consider using theget_available_demo_workspaces()helper.This logic duplicates what
get_available_demo_workspaces()already does. Using the helper would improve maintainability and ensure consistent behavior.♻️ Suggested simplification
- # Get available demo workspaces - demo_config = st.session_state.settings.get("demo_workspaces", {}) - demo_enabled = demo_config.get("enabled", False) - demo_source_dir = Path(demo_config.get("source_dir", "example-data/workspaces")) - available_demos = [] - if demo_enabled and demo_source_dir.exists(): - available_demos = [p.name for p in demo_source_dir.iterdir() if p.is_dir()] + # Get available demo workspaces + available_demos = get_available_demo_workspaces() + demo_config = st.session_state.settings.get("demo_workspaces", {}) + demo_source_dir = Path(demo_config.get("source_dir", "example-data/workspaces"))
509-523: Add error handling for file copy operations.The file copy operations can fail mid-operation, leaving the workspace in a partial/inconsistent state. Consider wrapping in try-except with user feedback.
♻️ Suggested improvement
if st.button("Load Demo Data"): demo_path = demo_source_dir / selected_demo if demo_path.exists(): - # Copy demo files to current workspace - for item in demo_path.iterdir(): - target = st.session_state.workspace / item.name - if item.is_dir(): - if target.exists(): - shutil.rmtree(target) - shutil.copytree(item, target) - else: - shutil.copy2(item, target) - st.success(f"Demo data '{selected_demo}' loaded!") - time.sleep(1) - st.rerun() + try: + # Copy demo files to current workspace + for item in demo_path.iterdir(): + target = st.session_state.workspace / item.name + if item.is_dir(): + if target.exists(): + shutil.rmtree(target) + shutil.copytree(item, target) + else: + shutil.copy2(item, target) + st.success(f"Demo data '{selected_demo}' loaded!") + time.sleep(1) + st.rerun() + except Exception as e: + st.error(f"Failed to load demo data: {e}")
496-500: Consider usingget_available_demo_workspaces()helper.This is the third place where demo workspace discovery logic is duplicated. Using the helper function would reduce duplication and ensure consistent behavior.
- Add get_demo_source_dirs() helper to normalize config and return list of existing directories - Add find_demo_workspace_path() to search directories in order - Update get_available_demo_workspaces() to collect demos from all directories (first occurrence wins for naming conflicts) - Update copy_demo_workspace() to use multi-directory lookup - Refactor page_setup() and render_sidebar() to use new helper functions - Change settings.json from source_dir (string) to source_dirs (array) - Maintain backward compatibility with legacy source_dir format This enables mounting external demo directories (e.g., Docker volumes) alongside bundled demos. Non-existent directories are silently skipped.
There was a problem hiding this comment.
Pull request overview
This PR migrates demo workspace handling from FLASHApp to the template, enabling derivative apps to ship with pre-configured example data. It adds configuration support, helper functions for workspace discovery and copying, and UI elements for loading demo data.
Changes:
- Added demo_workspaces configuration in settings.json with enabled flag and source directories
- Created four new helper functions for demo workspace management (get_demo_source_dirs, get_available_demo_workspaces, find_demo_workspace_path, copy_demo_workspace)
- Modified page_setup() to detect demo workspace requests via URL and auto-copy demos for local mode
- Added sidebar UI expander in online mode for users to load demo data into their current workspace
- Created example-data/workspaces/default/ with params.json containing example workflow parameters
Reviewed changes
Copilot reviewed 3 out of 7 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| src/common/common.py | Added four helper functions for demo workspace discovery and copying; modified page_setup() to handle demo workspace URL parameters and auto-copy for local mode; added sidebar UI for loading demo data in online mode |
| settings.json | Added demo_workspaces configuration section with enabled flag and source_dirs array |
| example-data/workspaces/default/params.json | Created example params.json with pre-configured workflow parameters and sample file selections |
Comments suppressed due to low confidence (1)
src/common/common.py:402
- The PR description mentions "auto-loading for online sessions when configured" but there's no implementation or configuration option for this feature. The demo_workspaces configuration in settings.json doesn't include an "auto_load" or similar option, and the code at lines 399-402 creates empty UUID workspaces for new online sessions instead of auto-loading a default demo. Consider adding this feature as described or updating the PR description to match the actual implementation.
elif st.session_state.location == "online":
workspace_id = str(uuid.uuid1())
st.session_state.workspace = Path(workspaces_dir, workspace_id)
st.query_params.workspace = workspace_id
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| if target_path.exists(): | ||
| shutil.rmtree(target_path) | ||
| shutil.copytree(demo_path, target_path) |
There was a problem hiding this comment.
The shutil.copytree function will copy all contents including hidden files and directories (those starting with '.'). This may include unwanted system files like .DS_Store or version control files like .git. Consider using the ignore parameter of shutil.copytree to filter out hidden files, similar to: shutil.copytree(demo_path, target_path, ignore=shutil.ignore_patterns('.*'))
| shutil.copytree(demo_path, target_path) | |
| shutil.copytree(demo_path, target_path, ignore=shutil.ignore_patterns(".*")) |
| def get_demo_source_dirs() -> list[Path]: | ||
| """ | ||
| Get list of demo workspace source directories from settings. | ||
|
|
||
| Supports both legacy 'source_dir' (string) and new 'source_dirs' (array) formats. | ||
| Non-existent directories are silently skipped. | ||
|
|
||
| Returns: | ||
| list[Path]: List of existing source directory paths. | ||
| """ | ||
| settings = st.session_state.get("settings", {}) | ||
| demo_config = settings.get("demo_workspaces", {}) | ||
|
|
||
| if not demo_config.get("enabled", False): | ||
| return [] | ||
|
|
||
| # Support both source_dirs (array) and source_dir (string) for backward compatibility | ||
| if "source_dirs" in demo_config: | ||
| dirs = demo_config["source_dirs"] | ||
| if isinstance(dirs, str): | ||
| dirs = [dirs] | ||
| elif "source_dir" in demo_config: | ||
| dirs = [demo_config["source_dir"]] | ||
| else: | ||
| dirs = ["example-data/workspaces"] | ||
|
|
||
| # Return only existing directories | ||
| return [Path(d) for d in dirs if Path(d).exists()] | ||
|
|
||
|
|
||
| def get_available_demo_workspaces() -> list[str]: | ||
| """ | ||
| Get a list of available demo workspaces from all configured source directories. | ||
|
|
||
| When the same demo name exists in multiple directories, the first occurrence wins. | ||
|
|
||
| Returns: | ||
| list[str]: List of unique demo workspace names. | ||
| """ | ||
| seen = set() | ||
| demos = [] | ||
|
|
||
| for source_dir in get_demo_source_dirs(): | ||
| for p in source_dir.iterdir(): | ||
| if p.is_dir() and p.name not in seen: | ||
| seen.add(p.name) | ||
| demos.append(p.name) | ||
|
|
||
| return demos | ||
|
|
||
|
|
||
| def find_demo_workspace_path(demo_name: str) -> Path | None: | ||
| """ | ||
| Find the source path for a demo workspace by searching all configured directories. | ||
|
|
||
| Directories are searched in order; the first match is returned. | ||
|
|
||
| Args: | ||
| demo_name: Name of the demo workspace to find. | ||
|
|
||
| Returns: | ||
| Path to the demo workspace, or None if not found. | ||
| """ | ||
| for source_dir in get_demo_source_dirs(): | ||
| demo_path = source_dir / demo_name | ||
| if demo_path.exists() and demo_path.is_dir(): | ||
| return demo_path | ||
| return None | ||
|
|
||
|
|
||
| def copy_demo_workspace(demo_name: str, target_path: Path) -> bool: | ||
| """ | ||
| Copy a demo workspace to the target path. | ||
|
|
||
| Searches all configured source directories for the demo (first match wins). | ||
|
|
||
| Args: | ||
| demo_name: Name of the demo workspace to copy. | ||
| target_path: Destination path for the workspace. | ||
|
|
||
| Returns: | ||
| bool: True if copy was successful, False otherwise. | ||
| """ | ||
| demo_path = find_demo_workspace_path(demo_name) | ||
|
|
||
| if demo_path is None: | ||
| return False | ||
|
|
||
| try: | ||
| if target_path.exists(): | ||
| shutil.rmtree(target_path) | ||
| shutil.copytree(demo_path, target_path) | ||
| return True | ||
| except Exception: | ||
| return False | ||
|
|
There was a problem hiding this comment.
The new demo workspace functionality (get_demo_source_dirs, get_available_demo_workspaces, find_demo_workspace_path, copy_demo_workspace) lacks test coverage. Given that the repository has comprehensive automated testing for Python (as seen in tests/test_simple_workflow.py and tests/test_topp_workflow_parameter.py), consider adding unit tests for these functions to verify behavior with various configurations, error cases, and edge conditions (e.g., non-existent directories, permission errors, path traversal attempts).
| shutil.copy2(item, target) | ||
| st.success(f"Demo data '{selected_demo}' loaded!") | ||
| time.sleep(1) | ||
| st.rerun() |
There was a problem hiding this comment.
If find_demo_workspace_path returns None (which could happen if the demo workspace is deleted between loading the UI and clicking the button), the user receives no feedback. Consider adding an else clause with an error message like "st.error(f'Demo workspace {selected_demo} not found')" to inform users when the demo data cannot be loaded.
| st.rerun() | |
| st.rerun() | |
| else: | |
| st.error(f"Demo workspace '{selected_demo}' not found") |
| demos = [] | ||
|
|
||
| for source_dir in get_demo_source_dirs(): | ||
| for p in source_dir.iterdir(): |
There was a problem hiding this comment.
The iterdir() method will raise an exception if the source directory is not accessible or has permission issues. Consider adding error handling to gracefully skip directories that cannot be read, similar to how get_demo_source_dirs() silently skips non-existent directories.
| for p in source_dir.iterdir(): | |
| try: | |
| entries = list(source_dir.iterdir()) | |
| except OSError: | |
| # Skip directories that cannot be accessed (e.g., permission issues) | |
| continue | |
| for p in entries: |
| def find_demo_workspace_path(demo_name: str) -> Path | None: | ||
| """ | ||
| Find the source path for a demo workspace by searching all configured directories. | ||
|
|
||
| Directories are searched in order; the first match is returned. | ||
|
|
||
| Args: | ||
| demo_name: Name of the demo workspace to find. | ||
|
|
||
| Returns: | ||
| Path to the demo workspace, or None if not found. | ||
| """ | ||
| for source_dir in get_demo_source_dirs(): | ||
| demo_path = source_dir / demo_name | ||
| if demo_path.exists() and demo_path.is_dir(): | ||
| return demo_path | ||
| return None |
There was a problem hiding this comment.
This function does not validate the demo_name parameter against path traversal attacks. A malicious user could potentially supply a demo_name like "../../../etc/passwd" to access files outside the intended demo directories. Consider validating that demo_name does not contain path separators or use Path.resolve() with a check that the resulting path is within the expected source directory.
| for item in demo_path.iterdir(): | ||
| target = st.session_state.workspace / item.name | ||
| if item.is_dir(): | ||
| if target.exists(): | ||
| shutil.rmtree(target) | ||
| shutil.copytree(item, target) | ||
| else: | ||
| shutil.copy2(item, target) | ||
| st.success(f"Demo data '{selected_demo}' loaded!") | ||
| time.sleep(1) | ||
| st.rerun() |
There was a problem hiding this comment.
The iterdir() loop does not handle potential exceptions during file operations. If an error occurs while copying individual files (e.g., permission issues, disk full, corrupted source files), the entire operation will fail silently without user feedback. Consider wrapping the file operations in a try-except block and providing meaningful error messages to the user.
| for item in demo_path.iterdir(): | |
| target = st.session_state.workspace / item.name | |
| if item.is_dir(): | |
| if target.exists(): | |
| shutil.rmtree(target) | |
| shutil.copytree(item, target) | |
| else: | |
| shutil.copy2(item, target) | |
| st.success(f"Demo data '{selected_demo}' loaded!") | |
| time.sleep(1) | |
| st.rerun() | |
| errors = [] | |
| for item in demo_path.iterdir(): | |
| target = st.session_state.workspace / item.name | |
| try: | |
| if item.is_dir(): | |
| if target.exists(): | |
| shutil.rmtree(target) | |
| shutil.copytree(item, target) | |
| else: | |
| shutil.copy2(item, target) | |
| except (OSError, shutil.Error) as exc: | |
| errors.append( | |
| f"Failed to copy '{item.name}' to workspace: {exc}" | |
| ) | |
| if errors: | |
| for msg in errors: | |
| st.error(msg) | |
| else: | |
| st.success(f"Demo data '{selected_demo}' loaded!") | |
| time.sleep(1) | |
| st.rerun() |
| st.session_state.workspace = Path( | ||
| workspaces_dir, requested_workspace | ||
| ) |
There was a problem hiding this comment.
The requested_workspace value from query parameters is used directly to construct a file path without validation. In local mode or for non-demo workspaces in online mode, this could allow path traversal attacks where an attacker could supply a value like "../../../sensitive-data" to access files outside the intended workspaces directory. Consider validating that requested_workspace doesn't contain path separators or use Path.resolve() with a check to ensure the resulting path is within workspaces_dir.
| st.session_state.workspace = Path( | |
| workspaces_dir, requested_workspace | |
| ) | |
| # Validate requested_workspace to prevent path traversal | |
| requested_workspace_path = Path(str(requested_workspace)) | |
| # Disallow path separators and parent directory references | |
| if ( | |
| requested_workspace_path.name != str(requested_workspace) | |
| or ".." in requested_workspace_path.parts | |
| ): | |
| safe_workspace_name = "default" | |
| else: | |
| base_dir = Path(workspaces_dir).resolve() | |
| candidate_workspace = (base_dir / requested_workspace_path.name).resolve() | |
| try: | |
| # Ensure the candidate path is within the base directory | |
| candidate_workspace.relative_to(base_dir) | |
| safe_workspace_name = requested_workspace_path.name | |
| except ValueError: | |
| safe_workspace_name = "default" | |
| st.session_state.workspace = Path(workspaces_dir, safe_workspace_name) | |
| st.query_params.workspace = safe_workspace_name |
| # For local mode with workspaces disabled, copy demo workspaces if they don't exist | ||
| for demo_name in get_available_demo_workspaces(): | ||
| target = Path(workspaces_dir, demo_name) | ||
| if not target.exists(): | ||
| copy_demo_workspace(demo_name, target) |
There was a problem hiding this comment.
This loop runs on every session initialization when workspaces are disabled in local mode. If there are many demo workspaces, this could slow down the initial page load. Consider moving this to a one-time initialization script or checking if the demos have already been copied to avoid repeated checks on every session.
| # For local mode with workspaces disabled, copy demo workspaces if they don't exist | |
| for demo_name in get_available_demo_workspaces(): | |
| target = Path(workspaces_dir, demo_name) | |
| if not target.exists(): | |
| copy_demo_workspace(demo_name, target) | |
| # For local mode with workspaces disabled, copy demo workspaces if they don't exist. | |
| # This can be expensive when many demo workspaces are present, so guard it with a | |
| # session-level flag to avoid repeating the checks on every session initialization. | |
| if ( | |
| st.session_state.location == "local" | |
| and not st.session_state.get("demo_workspaces_initialized", False) | |
| ): | |
| for demo_name in get_available_demo_workspaces(): | |
| target = Path(workspaces_dir, demo_name) | |
| if not target.exists(): | |
| copy_demo_workspace(demo_name, target) | |
| st.session_state["demo_workspaces_initialized"] = True |
| st.session_state.workspace = Path(workspaces_dir, workspace_id) | ||
| st.query_params.workspace = workspace_id | ||
| # Copy demo workspace contents using helper function | ||
| copy_demo_workspace(requested_workspace, st.session_state.workspace) |
There was a problem hiding this comment.
The success of the copy_demo_workspace operation is not checked. If the copy fails (returns False), the workspace will be created empty, which could lead to confusing behavior for users who expect the demo data to be present. Consider checking the return value and handling failure appropriately (e.g., showing an error message or falling back to an empty workspace with clear user feedback).
| for item in demo_path.iterdir(): | ||
| target = st.session_state.workspace / item.name | ||
| if item.is_dir(): | ||
| if target.exists(): | ||
| shutil.rmtree(target) | ||
| shutil.copytree(item, target) | ||
| else: | ||
| shutil.copy2(item, target) |
There was a problem hiding this comment.
The iterdir() loop copies all items from the demo workspace, including hidden files and directories (those starting with '.'), which may include system files like .DS_Store, .git, or other metadata files. Consider filtering out hidden files to avoid copying unwanted system or version control files into user workspaces.
- Rename example-data/workspaces/default to example_demo - Remove pre-loaded mzML files from demo workspace (~30MB savings) - Update params.json with empty file selection - Add .gitkeep to preserve empty mzML-files directory The demo workspace now serves as an empty template. Users can load example data via the existing "Load Example" button in fileupload.py which uses example-data/mzML/ as the source.
- Add is_safe_workspace_name() to reject names with path separators or parent directory references (/, \, .., .) - Apply validation in find_demo_workspace_path() to prevent demo name path traversal attacks - Apply validation in page_setup() for workspace query parameter, falling back to new UUID workspace if invalid Addresses Copilot security review comments #1 and #2.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/common/common.py`:
- Around line 45-73: get_demo_source_dirs currently filters demo paths by
Path(d).exists(), which lets files through and can cause NotADirectoryError when
callers like get_available_demo_workspaces or find_demo_workspace_path call
iterdir(); change the filter to only include directories by using
Path(d).is_dir() (keep the same handling of "source_dirs" vs "source_dir" and
default value), so the function returns only actual directories and prevents
iterdir() errors.
| def get_demo_source_dirs() -> list[Path]: | ||
| """ | ||
| Get list of demo workspace source directories from settings. | ||
|
|
||
| Supports both legacy 'source_dir' (string) and new 'source_dirs' (array) formats. | ||
| Non-existent directories are silently skipped. | ||
|
|
||
| Returns: | ||
| list[Path]: List of existing source directory paths. | ||
| """ | ||
| settings = st.session_state.get("settings", {}) | ||
| demo_config = settings.get("demo_workspaces", {}) | ||
|
|
||
| if not demo_config.get("enabled", False): | ||
| return [] | ||
|
|
||
| # Support both source_dirs (array) and source_dir (string) for backward compatibility | ||
| if "source_dirs" in demo_config: | ||
| dirs = demo_config["source_dirs"] | ||
| if isinstance(dirs, str): | ||
| dirs = [dirs] | ||
| elif "source_dir" in demo_config: | ||
| dirs = [demo_config["source_dir"]] | ||
| else: | ||
| dirs = ["example-data/workspaces"] | ||
|
|
||
| # Return only existing directories | ||
| return [Path(d) for d in dirs if Path(d).exists()] | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all Python files in the src/common directory and search for the relevant functions
fd "\.py$" src/common | head -20Repository: OpenMS/streamlit-template
Length of output: 112
🏁 Script executed:
# Search for get_available_demo_workspaces function
rg "get_available_demo_workspaces" -A 20Repository: OpenMS/streamlit-template
Length of output: 5814
🏁 Script executed:
# Also check how get_demo_source_dirs is used
rg "get_demo_source_dirs" -B 2 -A 2Repository: OpenMS/streamlit-template
Length of output: 821
🏁 Script executed:
# Check if settings.json exists
ls -la settings.json 2>/dev/null || echo "settings.json not found"Repository: OpenMS/streamlit-template
Length of output: 130
🏁 Script executed:
# Check the settings.json configuration
cat settings.jsonRepository: OpenMS/streamlit-template
Length of output: 781
🏁 Script executed:
# Let me also verify what happens when iterdir() is called on a file path
python3 <<'EOF'
from pathlib import Path
import tempfile
# Create a temporary file and try to call iterdir() on it
with tempfile.NamedTemporaryFile(suffix=".txt", delete=False) as f:
temp_file = Path(f.name)
try:
list(temp_file.iterdir())
except Exception as e:
print(f"Error type: {type(e).__name__}")
print(f"Error message: {e}")
finally:
temp_file.unlink()
EOFRepository: OpenMS/streamlit-template
Length of output: 155
🏁 Script executed:
# Search for find_demo_workspace_path to see the full context
rg "def find_demo_workspace_path" -A 15Repository: OpenMS/streamlit-template
Length of output: 906
🏁 Script executed:
# Test what error iterdir() raises on a file path
python3 <<'EOF'
from pathlib import Path
import tempfile
import os
# Create a temporary file
with tempfile.NamedTemporaryFile(mode='w', suffix=".txt", delete=False) as f:
temp_file_path = f.name
f.write("test content")
try:
p = Path(temp_file_path)
print(f"Path exists: {p.exists()}")
print(f"Path is_dir: {p.is_dir()}")
print(f"Attempting iterdir() on file...")
list(p.iterdir())
except Exception as e:
print(f"Error type: {type(e).__name__}")
print(f"Error message: {e}")
finally:
os.unlink(temp_file_path)
EOFRepository: OpenMS/streamlit-template
Length of output: 234
Filter demo source paths to directories to avoid iterdir() errors.
get_available_demo_workspaces() and find_demo_workspace_path() call iterdir() on paths returned from get_demo_source_dirs(), assuming each entry is a directory. If a file path is accidentally configured, iterdir() will raise NotADirectoryError. Filtering with is_dir() instead of exists() ensures robustness against misconfiguration.
♻️ Suggested change
- return [Path(d) for d in dirs if Path(d).exists()]
+ return [Path(d) for d in dirs if Path(d).is_dir()]🤖 Prompt for AI Agents
In `@src/common/common.py` around lines 45 - 73, get_demo_source_dirs currently
filters demo paths by Path(d).exists(), which lets files through and can cause
NotADirectoryError when callers like get_available_demo_workspaces or
find_demo_workspace_path call iterdir(); change the filter to only include
directories by using Path(d).is_dir() (keep the same handling of "source_dirs"
vs "source_dir" and default value), so the function returns only actual
directories and prevents iterdir() errors.
This migrates the demo workspace handling pattern from FLASHApp to the template, allowing derivative apps to ship with pre-configured example data that users can explore without uploading their own files.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.