Skip to content
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

Update README to run on Windows #1751

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

indiansinrichmondbc
Copy link

@indiansinrichmondbc indiansinrichmondbc commented Feb 8, 2025

Update the repository to include Windows setup instructions and scripts.

  • README.md: Add a section for Windows setup instructions, detailing the use of PowerShell scripts.
  • setup.ps1: Add a PowerShell script for Windows setup, including installation of dependencies, environment setup, and configuration of LLM providers.
  • docker-compose.yml: Add Windows-specific configurations and instructions.
  • entrypoint-skyvern.ps1: Add a PowerShell script for Windows entry point, including steps for starting Xvfb and running the server.
  • docs/development.mdx: Add Windows-specific setup instructions, including details for running PowerShell scripts.
  • docs/quickstart.mdx: Add Windows-specific setup instructions, including details for running PowerShell scripts.

For more details, open the Copilot Workspace session.


Important

Add Windows setup instructions and scripts, including PowerShell scripts and Docker configurations, to support Windows environments.

  • Documentation:
    • README.md: Add Windows setup instructions using PowerShell scripts.
    • docs/development.mdx and docs/quickstart.mdx: Add Windows-specific setup instructions.
  • Scripts:
    • setup.ps1: New PowerShell script for Windows setup, handling dependencies, environment setup, and LLM provider configuration.
    • entrypoint-skyvern.ps1: New PowerShell script for Windows entry point, starting Xvfb and running the server.
  • Configuration:
    • docker-compose.yml: Add Windows-specific service configurations for skyvern-windows and skyvern-ui-windows.

This description was created by Ellipsis for 5b82329. It will automatically update as commits are pushed.

Update the repository to include Windows setup instructions and scripts.

* **README.md**: Add a section for Windows setup instructions, detailing the use of PowerShell scripts.
* **setup.ps1**: Add a PowerShell script for Windows setup, including installation of dependencies, environment setup, and configuration of LLM providers.
* **docker-compose.yml**: Add Windows-specific configurations and instructions.
* **entrypoint-skyvern.ps1**: Add a PowerShell script for Windows entry point, including steps for starting Xvfb and running the server.
* **docs/development.mdx**: Add Windows-specific setup instructions, including details for running PowerShell scripts.
* **docs/quickstart.mdx**: Add Windows-specific setup instructions, including details for running PowerShell scripts.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/Skyvern-AI/skyvern?shareId=XXXX-XXXX-XXXX-XXXX).
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Reviewed everything up to 5b82329 in 3 minutes and 21 seconds

More details
  • Looked at 514 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 9 drafted comments based on config settings.
1. entrypoint-skyvern.ps1:13
  • Draft comment:
    Xvfb isn’t standard on Windows. Clarify if this depends on WSL or provide instructions for its installation.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
    The comment raises a valid technical concern - Xvfb is indeed not standard on Windows. However, this appears to be an entrypoint script, likely part of a larger system where the environment setup (including Xvfb) would be handled by installation/deployment scripts. The comment is asking for clarification rather than pointing out a clear code issue that needs to be fixed.
    The comment might be identifying a real deployment issue that could cause problems for Windows users. Installation instructions could be valuable.
    While valid, this kind of system requirement documentation belongs in README or documentation files, not in PR comments. The script itself is technically correct assuming its dependencies are met.
    Delete the comment because it's asking for clarification/documentation rather than pointing out a code issue that needs to be fixed.
2. setup.ps1:196
  • Draft comment:
    Ensure that the use of external commands like 'pg_isready', 'createuser', and 'createdb' are properly handled if not available on the system.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the author to ensure that certain external commands are properly handled if they are not available on the system. This is a request for confirmation or verification, which violates the rule against asking the author to ensure behavior is intended or tested.
3. README.md:138
  • Draft comment:
    Consider using sequential numbering (e.g., 1, 2, 3) instead of repeatedly using '1.' for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
4. docker-compose.yml:109
  • Draft comment:
    Windows service configurations duplicate common settings. Consider refactoring shared config into variables or anchors.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
5. entrypoint-skyvern.ps1:14
  • Draft comment:
    Xvfb is typically a Linux tool and may not be available on Windows. Verify that Xvfb is installed or provide a Windows alternative.
  • Reason this comment was not posted:
    Marked as duplicate.
6. setup.ps1:20
  • Draft comment:
    Review the regex update logic in UpdateOrAddEnvVar to ensure .env variables are correctly replaced in multiline files.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
7. setup.ps1:133
  • Draft comment:
    Validate the numeric input for the LLM model selection to handle unexpected or invalid responses.
  • Reason this comment was not posted:
    Marked as duplicate.
8. setup.ps1:250
  • Draft comment:
    Add error handling for token extraction in CreateOrganization in case the regex fails to match.
  • Reason this comment was not posted:
    Marked as duplicate.
9. setup.ps1:210
  • Draft comment:
    Consider more robust error handling for external commands (e.g., 'psql', 'pg_isready') in SetupPostgreSQL to gracefully handle missing tools or failures.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None

Workflow ID: wflow_A2NqXBFfmSNzsh7E


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

indiansinrichmondbc and others added 2 commits February 8, 2025 21:29
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Comment on lines +109 to +151
# Windows-specific configurations
skyvern-windows:
image: public.ecr.aws/skyvern/skyvern:latest
restart: on-failure
ports:
- 8000:8000
volumes:
- ./artifacts:/data/artifacts
- ./videos:/data/videos
- ./har:/data/har
- ./log:/data/log
- ./.streamlit:/app/.streamlit
environment:
- DATABASE_STRING=postgresql+psycopg://skyvern:skyvern@postgres:5432/skyvern
- BROWSER_TYPE=chromium-headful
- ENABLE_OPENAI=true
- LLM_KEY=OPENAI_GPT4O
- OPENAI_API_KEY=<your_openai_key>
depends_on:
postgres:
condition: service_healthy
healthcheck:
test: ["CMD", "test", "-f", "/app/.streamlit/secrets.toml"]
interval: 5s
timeout: 5s
retries: 5

skyvern-ui-windows:
image: public.ecr.aws/skyvern/skyvern-ui:latest
restart: on-failure
ports:
- 8080:8080
- 9090:9090
volumes:
- ./artifacts:/data/artifacts
- ./videos:/data/videos
- ./har:/data/har
- ./.streamlit:/app/.streamlit
environment:
- VITE_WSS_BASE_URL=ws://localhost:8000/api/v1
depends_on:
skyvern-windows:
condition: service_healthy
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need these container for windows? i can't see anything different from the linux container.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants