-
Notifications
You must be signed in to change notification settings - Fork 935
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
base: main
Are you sure you want to change the base?
Update README to run on Windows #1751
Conversation
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).
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.
❌ Changes requested. Reviewed everything up to 5b82329 in 3 minutes and 21 seconds
More details
- Looked at
514
lines of code in6
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%
<= threshold50%
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%
<= threshold50%
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%
<= threshold50%
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%
<= threshold50%
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%
<= threshold50%
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.
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>
# 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 |
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.
why do we need these container for windows? i can't see anything different from the linux container.
Update the repository to include Windows setup instructions and 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.
README.md
: Add Windows setup instructions using PowerShell scripts.docs/development.mdx
anddocs/quickstart.mdx
: Add Windows-specific setup instructions.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.docker-compose.yml
: Add Windows-specific service configurations forskyvern-windows
andskyvern-ui-windows
.This description was created by
for 5b82329. It will automatically update as commits are pushed.