Auto-create user/style.css if missing to fix issue #897#969
Open
Prathamesh8989 wants to merge 2 commits intofossasia:masterfrom
Open
Auto-create user/style.css if missing to fix issue #897#969Prathamesh8989 wants to merge 2 commits intofossasia:masterfrom
Prathamesh8989 wants to merge 2 commits intofossasia:masterfrom
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds automatic creation of a user/style.css file and directory at Visdom server startup and simplifies hostname/URL printing logic. Sequence diagram for Visdom server startup with auto-created user/style.csssequenceDiagram
actor User
participant CLI as CLI
participant Server as VisdomServer
participant FS as FileSystem
User->>CLI: run visdom
CLI->>Server: start_server()
Server->>FS: check existence of user directory
alt user directory missing
Server->>FS: create user directory
end
Server->>FS: check existence of style.css
alt style.css missing
Server->>FS: create empty style.css with header comment
end
Server->>Server: initialize Application
Server->>Server: configure hostname and base_url
Server->>User: print navigation URL
Server->>Server: start IOLoop
Flow diagram for start_server user/style.css auto-creation logicflowchart TD
A[start_server called] --> B[Compute user_dir as CWD/user]
B --> C{Does user_dir exist?}
C -- No --> D[Create user_dir]
C -- Yes --> E[Skip creating user_dir]
D --> F[Compute style_file as user_dir/style.css]
E --> F[Compute style_file as user_dir/style.css]
F --> G{Does style_file exist?}
G -- No --> H[Create style.css with header comment]
G -- Yes --> I[Skip creating style.css]
H --> J[Initialize Application]
I --> J[Initialize Application]
J --> K[Resolve hostname and base_url]
K --> L[Print navigation URL]
L --> M[Start IOLoop]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The auto-creation logic currently hardcodes
user_dirtoos.getcwd()/user, which doesn’t align with the PR description about using the environment path (env_path) and may placestyle.cssin an unexpected location relative to where Visdom actually looks for it; consider basing this path on the same env/config value the rest of the server uses. - The new auto-creation block could be simplified and made more robust by using
os.makedirs(user_dir, exist_ok=True)and perhaps creating an emptystyle.css(or one with a very minimal comment) to avoid unintentionally changing styling output in environments that serve this file directly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The auto-creation logic currently hardcodes `user_dir` to `os.getcwd()/user`, which doesn’t align with the PR description about using the environment path (`env_path`) and may place `style.css` in an unexpected location relative to where Visdom actually looks for it; consider basing this path on the same env/config value the rest of the server uses.
- The new auto-creation block could be simplified and made more robust by using `os.makedirs(user_dir, exist_ok=True)` and perhaps creating an empty `style.css` (or one with a very minimal comment) to avoid unintentionally changing styling output in environments that serve this file directly.
## Individual Comments
### Comment 1
<location> `py/visdom/server/run_server.py:44-45` </location>
<code_context>
+ # -------------------------
+ # Auto-create user/style.css
+ # -------------------------
+ user_dir = os.path.join(os.getcwd(), "user")
+ if not os.path.exists(user_dir):
+ os.makedirs(user_dir)
+
</code_context>
<issue_to_address>
**suggestion:** Consider using a more stable base directory than the current working directory for `user/style.css`.
Because `user_dir` depends on `os.getcwd()`, the location of `user/style.css` will vary based on how and where the process is started, potentially creating multiple `user` directories. Consider instead deriving this path from a fixed application root (for example, the module’s directory or a configured data directory) so the stylesheet location is consistent across runs.
Suggested implementation:
```python
def start_server(
port=DEFAULT_PORT,
hostname=DEFAULT_HOSTNAME,
bind_local=False,
eager_data_loading=False,
):
# -------------------------
# Auto-create user/style.css
# Use the module directory as a stable base instead of os.getcwd()
# -------------------------
module_dir = os.path.dirname(os.path.abspath(__file__))
user_dir = os.path.join(module_dir, "user")
if not os.path.exists(user_dir):
os.makedirs(user_dir, exist_ok=True)
def start_server(
port=DEFAULT_PORT,
hostname=DEFAULT_HOSTNAME,
bind_local=False,
eager_data_loading=False,
):
# -------------------------
# Auto-create user/style.css
# Use the module directory as a stable base instead of os.getcwd()
# -------------------------
module_dir = os.path.dirname(os.path.abspath(__file__))
user_dir = os.path.join(module_dir, "user")
if not os.path.exists(user_dir):
os.makedirs(user_dir, exist_ok=True)
```
1. Ensure `os` is imported at the top of `py/visdom/server/run_server.py` (if it is not already):
- Add `import os` alongside the other standard library imports.
2. If the intention is to create `user/style.css` itself (not just the directory), add logic after the `os.makedirs` call to create the file when it does not exist, e.g.:
```python
css_path = os.path.join(user_dir, "style.css")
if not os.path.exists(css_path):
with open(css_path, "w", encoding="utf-8") as f:
f.write("") # or default CSS contents
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Automatically creates the
user/style.cssfile during Visdom server startup if it doesn't exist. Also ensures theuserfolder is created inside the environment path (env_path). This prevents the “Style.css file not found” warning and allows users to add custom CSS overrides without manual folder creation.Motivation and Context
Fixes issue #897 ("Style.css file not found"). Previously, users had to manually create the
userfolder andstyle.cssafter installing Visdom. This change makes the process seamless, improving the out-of-the-box user experience.How Has This Been Tested?
userfolder.user/style.cssis automatically created in the correct environment path.Screenshots (if appropriate):
N/A — no UI changes by default; only auto-creation of a CSS file.
Types of changes
Checklist:
py/visdom/VERSIONaccording to Semantic VersioningSummary by Sourcery
Ensure the Visdom server automatically provisions a user CSS file and slightly tidy the server startup logging.
Bug Fixes:
Enhancements: