-
Notifications
You must be signed in to change notification settings - Fork 35
update Readme (Several way to run app) #267
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
Conversation
WalkthroughThe README.md file was extended with three new detailed sections: "Try the Online Demo," "Run Locally," and "Build with Docker." These sections provide step-by-step instructions for accessing the app online, setting it up locally with pip and streamlit, and deploying it using Docker, including installation, configuration, and usage details. Similarly, the installation documentation was restructured to unify local Python setup instructions and significantly enhance the Docker setup guide with explicit commands and verification steps. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (4)
README.md (4)
40-44: Skipped list index (3 ➜ 4)Markdown renders ordered lists correctly even when all items are
1., but using hard-coded numbers that jump from 2 → 4 may distract readers and invites bikeshedding in future edits.
Either switch to implicit1.numbering or fix the sequence.
45-46: Clarify “limited functionality”Stating that local runs are “limited” without concrete examples leaves readers guessing.
Briefly list at least one disabled feature (e.g., TOPP-tool execution, mzML processing) so users can decide which setup they need.
76-81: Minor wording & punctuation“Verify that Docker is working..” – double full stop.
Tiniest polish, but README is often the first impression.-Verify that Docker is working.. +Verify that Docker is working.
86-90: Mention access URL afterdocker compose upFirst-time users may not know Streamlit’s default port. Add an explicit hint:
docker compose up -d --build # App will be available at http://localhost:8501
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
README.md (1)
Learnt from: subCode321
PR: #160
File: content/run_example_workflow.py:83-88
Timestamp: 2025-03-10T11:09:56.467Z
Learning: In the OpenMS streamlit-template project, continuous real-time monitoring of CPU stats and workflow jobs is implemented using a while True loop, which is considered acceptable for this specific use case as confirmed by the maintainer.
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: 0
🧹 Nitpick comments (4)
README.md (4)
40-43: Fix skipped step number in “Run Locally” sectionThe list jumps from step 2 to step 4, which may confuse readers following the instructions sequentially.
-4. **Launch the app** +3. **Launch the app**
72-88: Renumber duplicated steps in “Build with Docker” sectionAfter step 4, the sequence restarts at 3.. Adjust to keep a strict ascending order.
-3. **Clone the repository** +3. **Clone the repository** … -4. **Specify GitHub token (to download Windows executables).** +4. **Specify GitHub token (to download Windows executables).** … -3. **Build & Launch the App** +5. **Build & Launch the App**
92-94: Use a single Compose command style
docker-compose(v1) anddocker compose(v2) are mixed in the same guide. Pick one form to avoid copy-paste errors—prefer the v2 CLI that ships with Docker ≥ 20.10:-docker-compose up -d --build +docker compose up -d --build
95-99: Add language identifiers to fenced blocksMarkdown-lint flags these blocks (MD040). Specify a language for proper syntax highlighting & quieter linters.
-``` +```text … -``` +```textAlso applies to: 103-106
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the openms streamlit-template project, continuous real-time monitoring of cpu stats and workflow ...
Learnt from: subCode321
PR: OpenMS/streamlit-template#160
File: content/run_example_workflow.py:83-88
Timestamp: 2025-03-10T11:09:56.467Z
Learning: In the OpenMS streamlit-template project, continuous real-time monitoring of CPU stats and workflow jobs is implemented using a while True loop, which is considered acceptable for this specific use case as confirmed by the maintainer.
Applied to files:
README.md
🪛 markdownlint-cli2 (0.17.2)
README.md
96-96: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
103-103: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (3)
- GitHub Check: build-openms
- GitHub Check: build-full-app
- GitHub Check: build-simple-app
🔇 Additional comments (1)
README.md (1)
108-112: Verify advertised vs. actual port mappingThe note says “map the port to default streamlit port
8501”, yet the command maps host8505→ container8501.docker run -p 8505:8501 openms_streamlit_templateConfirm whether the intended host port is 8501 or 8505 and align the note & command accordingly.
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: 0
🧹 Nitpick comments (4)
README.md (4)
25-45: Fix the out-of-sequence list numbering in “Run Locally”.The ordered list jumps from “2.” to “4.” which breaks visual flow (Markdown does not auto-renumber when explicit digits are used).
-4. **Launch the app** +3. **Launch the app**
78-88: Correct the step index reset in the Docker section.After “4. Specify GitHub token…”, the next heading is labelled “3.”.
Either rely on automatic1.numbering everywhere or advance to “5.”:-3. **Build & Launch the App** +5. **Build & Launch the App**
95-113: Add language identifiers to the fenced code blocks.
markdownlintflags these three blocks because ````` lacks a language tag.
Mark them as plain text (orbashwhere appropriate) to silence the linter and enable syntax highlighting:-``` +```text [+] Running 2/2 ✔ openms-streamlit-template Built ...Apply the same
text(orbash) marker to the blocks starting at Lines 104 and 110.
78-86: Guard against accidentally committing theGITHUB_TOKEN.Storing a personal access token in a
.envfile at the repo root is convenient but easy to mis-commit.
Add.env(or a more specific pattern) to.gitignore, and mention this safety step in the README.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the openms streamlit-template project, continuous real-time monitoring of cpu stats and workflow ...
Learnt from: subCode321
PR: OpenMS/streamlit-template#160
File: content/run_example_workflow.py:83-88
Timestamp: 2025-03-10T11:09:56.467Z
Learning: In the OpenMS streamlit-template project, continuous real-time monitoring of CPU stats and workflow jobs is implemented using a while True loop, which is considered acceptable for this specific use case as confirmed by the maintainer.
Applied to files:
README.md
🪛 markdownlint-cli2 (0.17.2)
README.md
96-96: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
103-103: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (3)
- GitHub Check: build-simple-app
- GitHub Check: build-full-app
- GitHub Check: build-openms
|
Looks good. |
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: 1
🧹 Nitpick comments (7)
docs/installation.md (3)
18-36: Numbering jumps (missing 3) – keep lists monotonic or use all 1..Readers see “1., 2., 4.” which looks like a typo even though Markdown auto-renumbers. Pick one style:
-4. **Launch the app** +3. **Launch the app**or simply write
1.for every list item.
48-65: Inconsistent CLI:docker-composevsdocker compose.The doc switches from the legacy hyphenated command when building (Line 91) to the modern sub-command (
docker compose ps, Line 100). Use one variant throughout to avoid confusion:-docker-compose up -d --build +docker compose up -d --buildSame change applies wherever the old spelling appears.
91-105: Code-block lint errors – add a language tag.Markdown-lint MD040 is raised for the console output blocks that start with “```” but lack a language hint (Lines 101, 108). Prefix them with e.g.
text:-``` +```text [+] Running 2/2Repeat for all unlabeled fenced blocks.
README.md (4)
25-38: Same numbering glitch as docs – 1, 2, 4.Keep the list monotonic or use all
1.for easier maintenance.-4. **Launch the app** +3. **Launch the app**
55-67: Shell snippet installs Linux packages only – clarify OS scope.The
<details>block removes old Docker versions viaapt-get. Mention that this is Ubuntu/Debian-specific to prevent copy-paste mishaps on other systems.
101-106: Add language tag to console output block (MD040).-``` +```text [+] Running 2/2
108-117: Second unlabeled block – same MD040 issue & port mismatch.
- Add
textlanguage tag.- The example maps host
8505→ container8501; maybe stick to8501:8501to match the “default streamlit port” wording.-docker run -p 8505:8501 openms_streamlit_template +docker run -p 8501:8501 openms_streamlit_template
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)docs/installation.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the openms streamlit-template project, continuous real-time monitoring of cpu stats and workflow ...
Learnt from: subCode321
PR: OpenMS/streamlit-template#160
File: content/run_example_workflow.py:83-88
Timestamp: 2025-03-10T11:09:56.467Z
Learning: In the OpenMS streamlit-template project, continuous real-time monitoring of CPU stats and workflow jobs is implemented using a while True loop, which is considered acceptable for this specific use case as confirmed by the maintainer.
Applied to files:
README.mddocs/installation.md
🪛 markdownlint-cli2 (0.17.2)
README.md
101-101: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
108-108: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/installation.md
101-101: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
108-108: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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: build-full-app
- GitHub Check: build-openms
🔇 Additional comments (1)
README.md (1)
17-24: Nice addition 👍 – online demo & local run sections read clearly.
| ## 💻 Run Locally | ||
|
|
||
| Create a virtual environment | ||
| To run the app locally: |
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.
🛠️ Refactor suggestion
Duplicate top-level sections – merge or drop “## 💻 Run Locally”.
A “## Python” section already exists (Line 10). Adding a second top-level “Run Locally” header fragments the flow and breaks the TOC. Either:
-## 💻 Run Locally
+### 💻 Run Locally <!-- as subsection of “## Python” -->or remove the earlier “## Python” header and keep the new one.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 💻 Run Locally | |
| Create a virtual environment | |
| To run the app locally: | |
| ### 💻 Run Locally <!-- as subsection of “## Python” --> | |
| To run the app locally: |
🤖 Prompt for AI Agents
In docs/installation.md around lines 14 to 16, there are duplicate top-level
headers "## Python" at line 10 and "## 💻 Run Locally" at line 14, which breaks
the table of contents and fragments the document flow. To fix this, either merge
the content under these two headers into a single top-level section or remove
one of the headers entirely to maintain a clear and consistent structure.
Summary by CodeRabbit